-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
module,src: add --experimental-entry-url
flag
#49975
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -617,6 +617,20 @@ files with no extension will be treated as WebAssembly if they begin with the | |
WebAssembly magic number (`\0asm`); otherwise they will be treated as ES module | ||
JavaScript. | ||
|
||
### `--experimental-entry-url` | ||
|
||
<!-- YAML | ||
added: | ||
- REPLACEME | ||
--> | ||
|
||
> Stability: 1.0 - Early development | ||
|
||
When present, Node.js will interpret the entry point as a URL, rather than a | ||
path. The URL must either start with `./` (e.g. `./entry.js`) or be absolute | ||
(e.g. `file:///home/user/entry.js`). Bare specifier (e.g. `entry.js`) won't | ||
work. | ||
Comment on lines
+630
to
+632
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This part sounds a bit vague: does "won't work" mean throwing an error, or being interpreted as relative URL? Can relative URL start with |
||
|
||
### `--experimental-import-meta-resolve` | ||
|
||
<!-- YAML | ||
|
@@ -2274,6 +2288,7 @@ Node.js options that are allowed are: | |
* `--enable-source-maps` | ||
* `--experimental-abortcontroller` | ||
* `--experimental-default-type` | ||
* `--experimental-entry-url` | ||
* `--experimental-import-meta-resolve` | ||
* `--experimental-json-modules` | ||
* `--experimental-loader` | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1028,6 +1028,7 @@ function throwIfInvalidParentURL(parentURL) { | |
*/ | ||
function defaultResolve(specifier, context = {}) { | ||
let { parentURL, conditions } = context; | ||
const isMain = parentURL === undefined; | ||
throwIfInvalidParentURL(parentURL); | ||
if (parentURL && policy?.manifest) { | ||
const redirects = policy.manifest.getDependencyMapper(parentURL); | ||
|
@@ -1083,8 +1084,8 @@ function defaultResolve(specifier, context = {}) { | |
) { | ||
return { __proto__: null, url: parsed.href }; | ||
} | ||
} catch { | ||
// Ignore exception | ||
} catch (err) { | ||
if (isMain && !shouldBeTreatedAsRelativeOrAbsolutePath(specifier)) { throw err; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please add a comment explaining this? |
||
} | ||
|
||
// There are multiple deep branches that can either throw or return; instead | ||
|
@@ -1102,7 +1103,6 @@ function defaultResolve(specifier, context = {}) { | |
if (parsed && parsed.protocol === 'node:') { return { __proto__: null, url: specifier }; } | ||
|
||
|
||
const isMain = parentURL === undefined; | ||
if (isMain) { | ||
parentURL = getCWDURL().href; | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,122 @@ | ||||||
import { spawnPromisified } from '../common/index.mjs'; | ||||||
import * as fixtures from '../common/fixtures.mjs'; | ||||||
import assert from 'node:assert'; | ||||||
import { execPath } from 'node:process'; | ||||||
import { describe, it } from 'node:test'; | ||||||
|
||||||
describe('--entry-url', { concurrency: true }, () => { | ||||||
it('should reject loading absolute path that contains %', async () => { | ||||||
const { code, signal, stderr, stdout } = await spawnPromisified( | ||||||
execPath, | ||||||
[ | ||||||
'--experimental-entry-url', | ||||||
fixtures.path('es-modules/test-esm-double-encoding-native%20.mjs'), | ||||||
] | ||||||
); | ||||||
|
||||||
assert.match(stderr, /ERR_MODULE_NOT_FOUND/); | ||||||
assert.strictEqual(stdout, ''); | ||||||
assert.strictEqual(code, 1); | ||||||
assert.strictEqual(signal, null); | ||||||
}); | ||||||
|
||||||
it('should support loading absolute Unix path properly encoded', async () => { | ||||||
const { code, signal, stderr, stdout } = await spawnPromisified( | ||||||
execPath, | ||||||
[ | ||||||
'--experimental-entry-url', | ||||||
fixtures.fileURL('es-modules/test-esm-double-encoding-native%20.mjs').pathname, | ||||||
] | ||||||
); | ||||||
|
||||||
assert.strictEqual(stderr, ''); | ||||||
assert.strictEqual(stdout, ''); | ||||||
assert.strictEqual(code, 0); | ||||||
assert.strictEqual(signal, null); | ||||||
}); | ||||||
|
||||||
it('should support loading absolute URLs', async () => { | ||||||
const { code, signal, stderr, stdout } = await spawnPromisified( | ||||||
execPath, | ||||||
[ | ||||||
'--experimental-entry-url', | ||||||
fixtures.fileURL('printA.js'), | ||||||
] | ||||||
); | ||||||
|
||||||
assert.strictEqual(stderr, ''); | ||||||
assert.match(stdout, /^A\r?\n$/); | ||||||
assert.strictEqual(code, 0); | ||||||
assert.strictEqual(signal, null); | ||||||
}); | ||||||
|
||||||
it('should support loading relative URLs', async () => { | ||||||
const { code, signal, stderr, stdout } = await spawnPromisified( | ||||||
execPath, | ||||||
[ | ||||||
'--experimental-entry-url', | ||||||
'./printA.js?key=value', | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't have to be in this PR, but we'll need to test |
||||||
], | ||||||
{ | ||||||
cwd: fixtures.fileURL('./'), | ||||||
} | ||||||
); | ||||||
|
||||||
assert.strictEqual(stderr, ''); | ||||||
assert.match(stdout, /^A\r?\n$/); | ||||||
assert.strictEqual(code, 0); | ||||||
assert.strictEqual(signal, null); | ||||||
}); | ||||||
|
||||||
it('should reject loading relative URLs without trailing `./`', async () => { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
const { code, signal, stderr, stdout } = await spawnPromisified( | ||||||
execPath, | ||||||
[ | ||||||
'--experimental-entry-url', | ||||||
'printA.js?key=value', | ||||||
], | ||||||
{ | ||||||
cwd: fixtures.fileURL('./'), | ||||||
} | ||||||
); | ||||||
|
||||||
assert.match(stderr, /ERR_INVALID_URL/); | ||||||
assert.strictEqual(stdout, ''); | ||||||
assert.strictEqual(code, 1); | ||||||
assert.strictEqual(signal, null); | ||||||
}); | ||||||
|
||||||
it('should reject loading bare specifiers from node_modules', async () => { | ||||||
const { code, signal, stderr, stdout } = await spawnPromisified( | ||||||
execPath, | ||||||
[ | ||||||
'--experimental-entry-url', | ||||||
'explicit-main', | ||||||
], | ||||||
{ | ||||||
cwd: fixtures.fileURL('./es-module-specifiers/'), | ||||||
} | ||||||
); | ||||||
|
||||||
assert.match(stderr, /ERR_INVALID_URL/); | ||||||
assert.strictEqual(stdout, ''); | ||||||
assert.strictEqual(code, 1); | ||||||
assert.strictEqual(signal, null); | ||||||
}); | ||||||
|
||||||
it('should support loading `data:` URLs', async () => { | ||||||
const { code, signal, stderr, stdout } = await spawnPromisified( | ||||||
execPath, | ||||||
[ | ||||||
'--experimental-entry-url', | ||||||
'data:text/javascript,console.log(0)', | ||||||
], | ||||||
); | ||||||
|
||||||
assert.strictEqual(stderr, ''); | ||||||
assert.match(stdout, /^0\r?\n$/); | ||||||
assert.strictEqual(code, 0); | ||||||
assert.strictEqual(signal, null); | ||||||
}); | ||||||
|
||||||
}); |
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 can call the flag
--entry-url
, with a printed experimental warning and the docs listing it as experimental. This isn’t a very risky feature that we need theexperimental
part in the flag name.