perf(core): use static specifier in ExtensionFileSource #18271
Conversation
| @@ -584,21 +567,21 @@ impl ExtensionBuilder { | |||
| #[cfg(not(feature = "include_js_files_for_snapshotting"))] | |||
| #[macro_export] | |||
| macro_rules! include_js_files { | |||
| (dir $dir:literal, $($file:literal,)+) => { | |||
| ($name:ident dir $dir:literal, $($file:literal,)+) => { | |||
There was a problem hiding this comment.
This change broke the existing code, which is expected. Unfortunately, the include_js_files! documentation is out of sync with the actual implementation, which makes it more difficult to fix this problem.
Current docs:
/// /// Example with "dir" option (for "my_extension"):
/// ```ignore
/// include_js_files!(
/// dir "js",
/// "01_hello.js",
/// "02_goodbye.js",
/// )
/// // Produces following specifiers:
/// - "ext:my_extension/js/01_hello.js"
/// - "ext:my_extension/js/02_goodbye.js"
/// ```
Produces compiler errors like this one:
error: couldn't read ext/mine/js: Is a directory (os error 21)
--> ext/mine/lib.rs:27:14
|
27 | .esm(include_js_files!(
| ______________^
28 | | dir "js",
29 | | "01_hello.js",
30 | | ))
| |_________^
|
= note: this error originates in the macro `include_str` which comes from the expansion of the macro `include_js_files` (in Nightly builds, run with -Z macro-backtrace for more info)
error: couldn't read ext/mine/01_hello.js: No such file or directory (os error 2)
--> ext/mine/lib.rs:27:14
|
27 | .esm(include_js_files!(
| ______________^
28 | | dir "js",
29 | | "01_hello.js",
30 | | ))
| |_________^
|
= note: this error originates in the macro `include_str` which comes from the expansion of the macro `include_js_files` (in Nightly builds, run with -Z macro-backtrace for more info)
There was a problem hiding this comment.
Sorry for that @bajtos. Could you help me spot here what's the mistake in docs?
There was a problem hiding this comment.
@bartlomieju sorry for posting a comment that's not actionable; I'll do better next time 🙈
The new version of include_js_files! requires the extension name ($name:ident) as the first argument.
I think this is the correct example:
include_js_files!(
"my-extension",
dir "js",
"01_hello.js",
"02_goodbye.js",
)In the end, I decided to bite the bullet and upgrade my code to use deno_core::extension! macro. It wasn't that much extra work, kudos for making the extension! macro easy to use outside of the Deno repo too!
There was a problem hiding this comment.
Ah I see, sorry again for the breaking change. We did all these changes to improve startup performance - both for Deno itself and all embedders. The extension! macro moves as much of the work to "build time" instead of "startup time" so hopefully you can see some reduction in your startup time too. Kudos go to @mmastrac for the macro :)
There was a problem hiding this comment.
The changes make sense; I understand why you are making them 👍🏻
Speaking about startup performance - is there any guide for embedders? I am particularly interested in learning how to set up our build to create a snapshot of all JS files we are bundling in our runtime to get that faster startup time you mentioned. I can probably figure this out by reading the source code of Deno core & cli, but it would be much nicer if there were a guide. Having said that, both the guide and snapshotting are pretty much "nice to have" for us right now.
There was a problem hiding this comment.
@bajtos there's no official guide - but long story short - you need to define extension! in your build.rs script when creating a snapshot and include all the files you want. Looking at runtime/build.rs and cli/build.rs (the latter creates a snapshot from existing snapshot) is the fastest way to see it in action. If you could open an issue about it we can look into adding some examples to deno_core.
Relanding #18264
Co-authored-by: Divy Srivastava [email protected]