-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: Use ES modules for internal runtime code #17648
Merged
Merged
Changes from 59 commits
Commits
Show all changes
67 commits
Select commit
Hold shift + click to select a range
925b649
work
crowlKats c6467d2
work
crowlKats 3df04f9
deno_url translated to esm
bartlomieju b31144b
deno_webgpu
crowlKats 3fe3908
deno_crypto translated to esm
bartlomieju 83aa1ec
deno_web
crowlKats c76cf84
websocket
crowlKats 22c2f3b
webstorage
crowlKats c28689f
fetch
crowlKats 39e2125
cache
crowlKats c1aa1f6
40_testing & 90_deno_ns
crowlKats 6617c40
40_testing & 90_deno_ns
crowlKats add5794
broadcast channel
crowlKats 708fe3d
ffi
crowlKats 33c0934
node
crowlKats dc7cbb9
net
crowlKats 663a5d1
deno_http translated to esm
bartlomieju 639b259
deno_flash translated to esm
bartlomieju f0e49cb
runtime
crowlKats 6235a01
runtime/41_prompt.js translated to esm
bartlomieju 10108b6
runtime/40_write_file.js translated to esm
bartlomieju 68dae03
tty, write_file
bartlomieju 07d36e6
make it compile
bartlomieju b491d3b
remove various globalThis.__bootstrap namespaces
bartlomieju a41a517
lint, remove debug logs
bartlomieju 0da5a8f
cleanup files inclusion
crowlKats b39ac46
use default export for primordials
crowlKats 3a714ab
fix
crowlKats 568fcec
export ops obj from 01_core.js
crowlKats e440e48
internals
crowlKats 49597de
internalsfix
crowlKats 9ab7c00
remove all __bootstrap usages
crowlKats d86ec77
fix webgpu
crowlKats ce4acc0
cleanup domexception
crowlKats f1ca615
Merge branch 'main' into es_builtins
crowlKats ea4376b
update imports
crowlKats 96e89f3
fmt
crowlKats f889210
clean up
crowlKats c40c26d
lint
crowlKats 5d36380
fix
crowlKats 35afbff
fix
crowlKats 0d8db69
fix
crowlKats 8429461
fix some tests
crowlKats de99562
add debug logs
crowlKats 7674d0e
fmt
crowlKats 043f96d
always use InternalModuleLoader
crowlKats 37eee91
Merge branch 'main' into es_builtins
crowlKats c8a3d70
fix & add test
crowlKats 5b79025
fmt
crowlKats fe5124f
fix one panic
bartlomieju 6437dee
fallback to regular loader
bartlomieju 10ce27a
fix tests for modules
bartlomieju 14ace36
revert core
crowlKats de0a21d
fix
crowlKats c1e9b5d
fix tests
crowlKats eee46d8
cleanup unwraps
crowlKats 4078801
fix interna.d.ts files
crowlKats 2287ad5
address comments
crowlKats 6be1fd3
cleanup InternalModuleLoader usage
crowlKats 8123feb
fix test
crowlKats 23bf8de
fix typings
crowlKats 8818a91
reject dynamic imports for internal modules
bartlomieju 6d091cb
Merge branch 'main' into es_builtins
bartlomieju 665c5c7
lint
crowlKats f823a16
fix std
crowlKats 8d622e8
fix streams
crowlKats 769273f
add InternalModuleLoader test
crowlKats File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
await import("internal:runtime/js/01_build.js"); |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
error: Uncaught (in promise) TypeError: Cannot load internal module from external code | ||
await import("internal:runtime/js/01_build.js"); | ||
^ | ||
at async [WILDCARD] |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
import "internal:runtime/js/01_build.js"; |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
error: Unsupported scheme "internal" for module "internal:runtime/js/01_build.js". Supported schemes: [ | ||
"data", | ||
"blob", | ||
"file", | ||
"http", | ||
"https", | ||
] | ||
at [WILDCARD] |
This file contains 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
This file contains 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does CLI get to the point of this error? This error is thrown by the internal module loader, not the cli loader. What is up with that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point @crowlKats, we shouldn't get an error from the CLI loader, not the internal loader.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this resolved? AFAICT, the error is still coming from the internal loader. Why is the internal loader used at runtime in CLI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lucacasonato it's not coming from internal loader - I disabled dynamic imports of internal modules completely in
core/bindings.rs
- so we're not even hitting module system, nor any of the loaders.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do the same for resolution in static loading?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static loading is erroring before currently, because the FileFetcher is erroring with invalid scheme
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could do that somewhere in
core/modules.rs
- essentially you're asking to hard disable loading of internal modules in cases where we are running from a snapshot?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we already are doing that though, no? the InternalModuleLoader isnt used by cli; its not used if one is loading a snapshot but not creating one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do the enforcement at core level in a follow up as Bartek described. CLI is not the only loader implementation. Not all loaders enforce a given scheme during resolution.