Fix module loading #798
Merged
Fix module loading #798
Conversation
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
When V8 calls the ResolveModuleCallback that we give it, it passes the specifier
which is essentially the string given to `from`:
```
import {x} from './blah.js';
```
We were taking that specifier and giving it to the page. The page knew the
currently executing script, an thus could resolve the full URL. Given the full
URL, it could either return the JS content from its module cache or fetch
the source.
At best though, this isn't efficient. If two files import the same module, yes
we cache the src, but we still ask v8 to re-compile it. At worse, it crashes
due to resource exhaustion in the case of cyclical dependencies.
ResolveModuleCallback should instead detect that it has already loaded the
module and return the previously loaded module. Essentially, we shouldn't be
caching the JavaScript source, we should be caching the v8 module.
However, in order to do this, we need more than the specifier, which might only
be a relative path (and thus isn't unique). So, in addition to a module cache,
we now also maintain an module identifier lookup. Given a module, we can get
its full path. Thankfully ResolveModuleCallback gives us the referring module,
so we can look up that modules URL, stitch it to the specifier, and get the
full url (the unique identifier) within the JS runtime.
Need more real world testing, and a fully working example before I celebrate,
but for sites with many import, this appears to improve performance by many
orders of magnitude.
Collaborator
Author
krichprollsch
approved these changes
Jun 23, 2025
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
When V8 calls the ResolveModuleCallback that we give it, it passes the specifier which is essentially the string given to
from:We were taking that specifier and giving it to the page. The page knew the currently executing script, an thus could resolve the full URL. Given the full URL, it could either return the JS content from its module cache or fetch the source.
At best though, this isn't efficient. If two files import the same module, yes we cache the src, but we still ask v8 to re-compile it. At worse, it crashes due to resource exhaustion in the case of cyclical dependencies.
ResolveModuleCallback should instead detect that it has already loaded the module and return the previously loaded module. Essentially, we shouldn't be caching the JavaScript source, we should be caching the v8 module.
However, in order to do this, we need more than the specifier, which might only be a relative path (and thus isn't unique). So, in addition to a module cache, we now also maintain an module identifier lookup. Given a module, we can get its full path. Thankfully ResolveModuleCallback gives us the referring module, so we can look up that modules URL, stitch it to the specifier, and get the full url (the unique identifier) within the JS runtime.
Need more real world testing, and a fully working example before I celebrate, but for sites with many import, this appears to improve performance by many orders of magnitude.