-
Notifications
You must be signed in to change notification settings - Fork 303
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
Disable top-level await in require with a compat flag #3081
Conversation
After an initial review of impact on this, I think this PR needs to be updated to relax the restriction a bit more. TLA should be restricted only when modules are |
46fa8bc
to
51d5c9a
Compare
9fcfd0b
to
23b4340
Compare
Updated to further refine and also eliminates the limitation around circular dependencies of commonjs modules. Not marking this "ready for review" just yet because there are a few more additional cleanups I want to do to eliminate duplicated code. |
This comment was marked as resolved.
This comment was marked as resolved.
Those will not be impacted. Note that this PR has been updated to ONLY apply to use of |
1f182f4
to
57cfd68
Compare
57cfd68
to
cfca7ac
Compare
cfca7ac
to
e858332
Compare
Currently, the top level scope of a Worker script and modules that are imported may use top level await. However, because we restrict the use of i/o in the top level scope, the only thing that can be awaited at the top level scope is a dynamic import, which we require to be resolved synchronously. It is a bit nonsensical to support synchronously resolved dynamic imports at the top level when we can just use static imports more effectively. Additionally, the ecosystem and runtimes are moving to a state where top level await in modules is being strongly discouraged. This compat flag. This compatibility flag will make it impossible to use top level await.(made the change with lots of distractions so keeping as a draft until I can do a second pass to make sure nothing was missed)
Update: Updated to apply the compat flag only to
require()
calls, along with a few other relevant cleanups. Still keeping as a draft because this will need some extensive testing.