| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Sorry, something went wrong.
|
I might not be reading the code correctly, but does this cover the case of: await Promise.resolve();
import 'x';It looks like the C++ code would try to handle the "await" syntax error first, wrap the code in an async function and recompile. The recompilation would then throw an "import" syntax error, ultimately causing should_retry_as_esm to remain false. |
Sorry, something went wrong.
Great catch @chjj, I’ve updated the PR to address that case. Any others that you can think of that I might have missed? |
Sorry, something went wrong.
There was a problem hiding this comment.
This is looking good! const module = 'x' as defining an ES module as an extension of the ESM syntax checks seems like the right kind of compromise to be making in the design space to me. I do hope we could optimize TLA in due course.
Note that when sloppy mode errors precede an identifier redeclaration there will still be error consistency, but as discussed separately there are ways to handle this by pointing to the correct error for the ESM parse, and this can be done as follow-up work if needed.
I would be happy to mark for approval as soon as we've got an updated note in the ESM docs describing the new format detection rules, as I think it's important to track the rules and their changes carefully.
Sorry, something went wrong.
Added. |
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
…round async function wrapper; add test
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
PR-URL: #52024 Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
PR-URL: #52024 Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
| Back | FazBrowse Home | New Git URL |
Fixes #50917, using the solution described in #50917 (comment).
The general algorithm for --experimental-detect-module is to try to parse as CommonJS, and if a syntax error is thrown that corresponds to ESM syntax (import or export, or import.meta), try again as ESM. The edge case pointed out by #50917 is when there’s syntax that throws in CommonJS but parses in ESM, and this syntax is above the first ESM syntax (so top-level await, or a declaration of a variable with the same name as one of the CommonJS module wrapper variables such as const require =, on the first line or any line above the first import or export).
The tricky thing is that the errors thrown by top-level await or const require in CommonJS are the same errors as thrown by typing await in any ordinary sync function, or by declaring require twice in user code (e.g. const require = 1; const require = 2). So we only want to retry parsing as ESM when these errors are because the await or problematic variable declaration is at the top level, where it throws in CommonJS but parses in ESM.
To determine this, this PR creates a new error path where if the CommonJS parse returns a syntax error corresponding to either of these cases, we do a special second CommonJS parse of the code wrapped in an async function. This wrapper function creates a new scope, so const require is no longer a problematic redeclaration; and await no longer throws because it’s within an async function. This wrapper only affects the top level, so await in a sync function farther down or variable redeclarations farther down (or two const declarations at the top level) will still throw; but the code permitted in ESM parses successfully. If this second parse doesn’t throw any errors, we resume the detection algorithm and try parsing as ESM.
@nodejs/loaders @chjj @meyfa @joyeecheung