feat(modules): async loader for dynamic import()#1522
Open
regularkevvv wants to merge 1 commit into
Open
Conversation
Add JS_SetModuleLoaderFuncAsync together with JS_FulfillAsyncModuleLoad and JS_RejectAsyncModuleLoad so embedders can satisfy import(specifier) from an asynchronous source (e.g. a network fetch) without blocking the JS thread. Scope is limited to dynamic import(); static imports keep using the synchronous JSModuleLoaderFunc[2], mirroring the HTML spec split between HostImportModuleDynamically and HostResolveImportedModule. The loader is handed an opaque handle and settles it later. Properties: - the specifier is normalized and a cached module is settled without calling the loader; - concurrent import()s of the same specifier share one load: the loader runs once and the module is evaluated once; - settling a handle more than once is a no-op, and handles still in flight are reclaimed by JS_FreeRuntime. Tested in api-test.c: fulfill, reject, transitive static import, cache hit, in-flight dedup, top-level await, re-entrant settle, attribute and normalizer hooks, nested dynamic import, and shutdown sweeps.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds
JS_SetModuleLoaderFuncAsync,JS_FulfillAsyncModuleLoad, andJS_RejectAsyncModuleLoadso embedders can satisfyimport(specifier)from an asynchronous source (e.g. a network fetch) without blocking the JS thread. The loader receives an opaque handle and settles it later.Addresses
#196 — "make dynamic import actually async" (primary):
SetHostImportModuleDynamicallyCallback, so "loading a dynamic module shouldn't itself be blocking" and can "integrate with nonblocking I/O … associate the Promise with our nonblocking I/O operation and, on completion, resolve the promise." → exactly this API. The scaling concern raised there — "Some websites do hundreds of dynamic async imports" — is what motivated the in-flight dedup + concurrency tests below.import()only.#191 — "async es module loader" (this PR is the dynamic half):
JS_Fulfill/RejectAsyncModuleLoadis precisely that "complete it later" API.js_dynamic_import_jobso dynamic import can be async (the sync path "will only work[] with sync module loading mode"). → that's the integration point used here.Scope / compatibility
Dynamic
import()only; static imports keep using the synchronousJSModuleLoaderFunc[2], mirroring the HTML spec split betweenHostImportModuleDynamicallyandHostResolveImportedModule. No behavior change and no cost when no async loader is registered; the existing loader union is untouched, so embedder ABI is unchanged.Design notes
import()s of the same specifier are de-duplicated: the loader runs once and the module is evaluated once via a single-eval fan-out, so all callers resolve to the same namespace.JS_FreeRuntimewithout running JS during teardown.Note on handle vs. returned Promise
The loader gets an opaque handle to settle later (
JS_Fulfill/RejectAsyncModuleLoad), rather than returning a JS Promise like V8'sSetHostImportModuleDynamicallyCallback— the shape referenced here. JS behavior is identical; only the C-facing shape differs. I went with the handle because it keeps C embedders from having to build and refcountPromiseobjects, and because it makes in-flight dedup natural: concurrentimport()s of one specifier attach as extra waiters on a single handle (one load, one evaluation).Glad to switch to the returned-Promise shape if you'd prefer — it's a localized change to the loader signature and
js_dynamic_import_job, and doesn't touch the dedup/shutdown machinery.Reviewer concern → test that asserts it
All in
api-test.c, run underJS_ABORT_ON_LEAKS; the whole binary also passes under ASAN+UBSAN.inflight_dedup— loader called once, evaluated once,import()===import()concurrent_distinct— 64 concurrent, 64 loads/64 evalsfulfill/reject→ UAF/double-freedouble_settleshutdown_pendingshutdown_multi_waitertransitive— static dep goes through the sync loaderawait(explicitly doubted in #191)tlareentrantattrs_reject— rejects before the loader runsnormalizeimport()from an async modulenested_dynamicNULLmodule → clean rejection (not crash)null_modulecache_hitreject,eval_throwTest results
make test(test262)0/63 errors·make ctest(C11, bothJS_NAN_BOXINGmodes) ·make cxxtest(C++11, both modes) ·api-testclean under ASAN+UBSAN — all green, no new warnings.