Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
esm: detect ESM syntax in ambiguous JavaScript#50096
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
5cc2ee58d6d67547d7f9ed0dd9937d642ec333d9b06e81e23b9af116File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| 'use strict' | ||
| // This benchmarks the cost of running `containsModuleSyntax` on a CommonJS module being imported. | ||
| // We use the TypeScript fixture because it's a very large CommonJS file with no ESM syntax: the worst case. | ||
| const common = require('../common.js'); | ||
| const tmpdir = require('../../test/common/tmpdir.js'); | ||
| const fixtures = require('../../test/common/fixtures.js'); | ||
| const scriptPath = fixtures.path('snapshot', 'typescript.js'); | ||
| const fs = require('node:fs'); | ||
| const bench = common.createBenchmark(main,{ | ||
| type: ['with-module-syntax-detection', 'without-module-syntax-detection'], | ||
| n: [1e4], | ||
| },{ | ||
| flags: ['--experimental-detect-module'], | ||
| }); | ||
| const benchmarkDirectory = tmpdir.fileURL('benchmark-detect-esm-syntax'); | ||
| const ambiguousURL = new URL('./typescript.js', benchmarkDirectory); | ||
| const explicitURL = new URL('./typescript.cjs', benchmarkDirectory); | ||
| async function main({n, type }){ | ||
| tmpdir.refresh(); | ||
| fs.mkdirSync(benchmarkDirectory,{recursive: true }); | ||
| fs.cpSync(scriptPath, ambiguousURL); | ||
| fs.cpSync(scriptPath, explicitURL); | ||
| bench.start(); | ||
| for (let i = 0; i < n; i++){ | ||
| const url = type === 'with-module-syntax-detection' ? ambiguousURL : explicitURL; | ||
| await import(url); | ||
| } | ||
| bench.end(n); | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -33,7 +33,7 @@ const DATA_URL_PATTERN = /^[^/]+\/[^,;]+(?:[^,]*?)(;base64)?,([\s\S]*)$/; | ||||||||||||||
| /** | ||||||||||||||
| * @param{URL} url URL to the module | ||||||||||||||
| * @param{ESModuleContext} context used to decorate error messages | ||||||||||||||
| * @returns{{responseURL: string, source: string | BufferView }} | ||||||||||||||
| * @returns{Promise<{responseURL: string, source: string | BufferView }>} | ||||||||||||||
| */ | ||||||||||||||
| async function getSource(url, context){ | ||||||||||||||
| const{protocol, href } = url; | ||||||||||||||
| @@ -105,7 +105,7 @@ function getSourceSync(url, context){ | ||||||||||||||
| * @param{LoadContext} context | ||||||||||||||
| * @returns{LoadReturn} | ||||||||||||||
| */ | ||||||||||||||
| async function defaultLoad(url, context = kEmptyObject){ | ||||||||||||||
| async function defaultLoad(url, context = {__proto__: null }){ | ||||||||||||||
| let responseURL = url; | ||||||||||||||
| let{ | ||||||||||||||
| importAttributes, | ||||||||||||||
| @@ -127,19 +127,24 @@ async function defaultLoad(url, context = kEmptyObject){ | ||||||||||||||
| throwIfUnsupportedURLScheme(urlInstance, experimentalNetworkImports); | ||||||||||||||
| format ??= await defaultGetFormat(urlInstance, context); | ||||||||||||||
| validateAttributes(url, format, importAttributes); | ||||||||||||||
| if ( | ||||||||||||||
| format === 'builtin' || | ||||||||||||||
| format === 'commonjs' | ||||||||||||||
| ){ | ||||||||||||||
| if (urlInstance.protocol === 'node:'){ | ||||||||||||||
| source = null; | ||||||||||||||
| } else if (source == null){ | ||||||||||||||
| ({responseURL, source } = await getSource(urlInstance, context)); | ||||||||||||||
| context.source = source; | ||||||||||||||
| } | ||||||||||||||
| if (format == null || format === 'commonjs'){ | ||||||||||||||
| // Now that we have the source for the module, run `defaultGetFormat` again in case we detect ESM syntax. | ||||||||||||||
| format = await defaultGetFormat(urlInstance, context); | ||||||||||||||
| } | ||||||||||||||
Comment on lines +137 to +140 Contributor
| ||||||||||||||
| if(format==null||format==='commonjs'){ | |
| // Now that we have the source for the module, run `defaultGetFormat` again in case we detect ESM syntax. | |
| format=awaitdefaultGetFormat(urlInstance,context); | |
| } | |
| // Now that we have the source for the module, run `defaultGetFormat` again in case we detect ESM syntax. | |
| format??=awaitdefaultGetFormat(urlInstance,context); |
Uh oh!
There was an error while loading. Please reload this page.
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.
This only supports string source, that's a bit annoying. Why can't we pass a
Uint8Arrayto the C++ binding?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'm not opposed if you can get that to work.
compileFunctionexpects a string though.