diff --git a/doc/api/esm.md b/doc/api/esm.md index 838e0810012ad5..17164959ca6205 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -800,6 +800,15 @@ imports and they cannot be inspected via `WebAssembly.Module.imports(mod)` or virtualized unless recompiling the module using the direct `WebAssembly.compile` API with string builtins disabled. +String constants may also be imported from the `wasm:js/string-constants` builtin +import URL, allowing static JS string globals to be defined: + +```text +(module + (import "wasm:js/string-constants" "hello" (global $hello externref)) +) +``` + Importing a module in the source phase before it has been instantiated will also use the compile-time builtins automatically: diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 929577c0da6d08..22032f79e90d44 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -145,7 +145,9 @@ class ModuleJobBase { */ syncLink(requestType) { // Store itself into the cache first before linking in case there are circular - // references in the linking. + // references in the linking. Track whether we're overwriting an existing entry + // so we know whether to remove the temporary entry in the finally block. + const hadPreviousEntry = this.loader.loadCache.get(this.url, this.type) !== undefined; this.loader.loadCache.set(this.url, this.type, this); const moduleRequests = this.module.getModuleRequests(); // Modules should be aligned with the moduleRequests array in order. @@ -169,9 +171,14 @@ class ModuleJobBase { } this.module.link(modules); } finally { - // Restore it - if it succeeds, we'll reset in the caller; Otherwise it's - // not cached and if the error is caught, subsequent attempt would still fail. - this.loader.loadCache.delete(this.url, this.type); + if (!hadPreviousEntry) { + // Remove the temporary entry. On failure this ensures subsequent attempts + // don't return a broken job. On success the caller + // (#getOrCreateModuleJobAfterResolve) will re-insert under the correct key. + this.loader.loadCache.delete(this.url, this.type); + } + // If there was a previous entry (ensurePhase() path), leave this in cache - + // it is the upgraded job and the caller will not re-insert. } return evaluationDepJobs; diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index d6c96996a900da..4643300041a638 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -523,6 +523,7 @@ translators.set('wasm', function(url, translateContext) { try { compiled = new WebAssembly.Module(source, { builtins: ['js-string'], + importedStringConstants: 'wasm:js/string-constants', }); } catch (err) { err.message = errPath(url) + ': ' + err.message; diff --git a/test/es-module/test-esm-wasm-source-phase-identity.mjs b/test/es-module/test-esm-wasm-source-phase-identity.mjs new file mode 100644 index 00000000000000..2d742dfcff61ab --- /dev/null +++ b/test/es-module/test-esm-wasm-source-phase-identity.mjs @@ -0,0 +1,11 @@ +// Regression test for source phase import identity with mixed eval/source +// phase imports of the same module in one parent. +import '../common/index.mjs'; +import { spawnSyncAndAssert } from '../common/child_process.js'; +import * as fixtures from '../common/fixtures.mjs'; + +spawnSyncAndAssert( + process.execPath, + ['--no-warnings', fixtures.path('es-modules/test-wasm-source-phase-identity.mjs')], + { stdout: '', stderr: '', trim: true } +); diff --git a/test/fixtures/es-modules/js-string-builtins.wasm b/test/fixtures/es-modules/js-string-builtins.wasm index b4c08587dd08e7..fe520bab1fbbf5 100644 Binary files a/test/fixtures/es-modules/js-string-builtins.wasm and b/test/fixtures/es-modules/js-string-builtins.wasm differ diff --git a/test/fixtures/es-modules/js-string-builtins.wat b/test/fixtures/es-modules/js-string-builtins.wat index 9bc55a8fa750cc..08f4ade8c4151c 100644 --- a/test/fixtures/es-modules/js-string-builtins.wat +++ b/test/fixtures/es-modules/js-string-builtins.wat @@ -4,11 +4,15 @@ (import "wasm:js-string" "length" (func $string_length (param externref) (result i32))) (import "wasm:js-string" "concat" (func $string_concat (param externref externref) (result (ref extern)))) (import "wasm:js-string" "equals" (func $string_equals (param externref externref) (result i32))) + + ;; Import a string constant via importedStringConstants + (import "wasm:js/string-constants" "hello" (global $hello externref)) ;; Export functions that use the builtins (export "getLength" (func $get_length)) (export "concatStrings" (func $concat_strings)) (export "compareStrings" (func $compare_strings)) + (export "getHello" (func $get_hello)) (func $get_length (param $str externref) (result i32) local.get $str @@ -26,4 +30,8 @@ local.get $str2 call $string_equals ) + + (func $get_hello (result externref) + global.get $hello + ) ) \ No newline at end of file diff --git a/test/fixtures/es-modules/test-wasm-js-string-builtins.mjs b/test/fixtures/es-modules/test-wasm-js-string-builtins.mjs index 2364f246b2558d..c76dcc39894932 100644 --- a/test/fixtures/es-modules/test-wasm-js-string-builtins.mjs +++ b/test/fixtures/es-modules/test-wasm-js-string-builtins.mjs @@ -6,3 +6,4 @@ strictEqual(wasmExports.getLength('hello'), 5); strictEqual(wasmExports.concatStrings('hello', ' world'), 'hello world'); strictEqual(wasmExports.compareStrings('test', 'test'), 1); strictEqual(wasmExports.compareStrings('test', 'different'), 0); +strictEqual(wasmExports.getHello(), 'hello'); diff --git a/test/fixtures/es-modules/test-wasm-source-phase-identity-parent.mjs b/test/fixtures/es-modules/test-wasm-source-phase-identity-parent.mjs new file mode 100644 index 00000000000000..36d5765c17f0ad --- /dev/null +++ b/test/fixtures/es-modules/test-wasm-source-phase-identity-parent.mjs @@ -0,0 +1,6 @@ +import * as mod1 from './simple.wasm'; +import * as mod2 from './simple.wasm'; +import source mod3 from './simple.wasm'; +import source mod4 from './simple.wasm'; + +export { mod1, mod2, mod3, mod4 }; diff --git a/test/fixtures/es-modules/test-wasm-source-phase-identity.mjs b/test/fixtures/es-modules/test-wasm-source-phase-identity.mjs new file mode 100644 index 00000000000000..84cf6261139038 --- /dev/null +++ b/test/fixtures/es-modules/test-wasm-source-phase-identity.mjs @@ -0,0 +1,14 @@ +import { strictEqual } from 'node:assert'; + +// Pre-load simple.wasm at kSourcePhase to prime the loadCache. +const preloaded = await import.source('./simple.wasm'); +strictEqual(preloaded instanceof WebAssembly.Module, true); + +// Import a parent that has both eval-phase and source-phase imports of the +// same wasm file, which triggers ensurePhase(kEvaluationPhase) on the cached +// job and exposes the loadCache eviction bug. +const { mod1, mod2, mod3, mod4 } = + await import('./test-wasm-source-phase-identity-parent.mjs'); + +strictEqual(mod1, mod2, 'two eval-phase imports of the same wasm must be identical'); +strictEqual(mod3, mod4, 'two source-phase imports of the same wasm must be identical');