feat: support import.meta.dirname and import.meta.filename #14854
Conversation
✅ Deploy Preview for jestjs ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -685,19 +697,22 @@ export default class Runtime { | |||
| specifier = fileURLToPath(specifier); | |||
| } | |||
|
|
|||
| const [path, query] = specifier.split('?'); | |||
| const [specifierPath, query] = specifier.split('?'); | |||
There was a problem hiding this comment.
NOTE: Conflicting name with imported node:path.
packages/jest-runtime/src/index.ts
Outdated
| @@ -520,6 +520,11 @@ export default class Runtime { | |||
| initializeImportMeta: (meta: JestImportMeta) => { | |||
| meta.url = pathToFileURL(modulePath).href; | |||
|
|
|||
| if (meta.url.startsWith('file://')) { | |||
There was a problem hiding this comment.
NOTE: The check here is because of the CAVEAT mention in nodejs docs (see https://nodejs.org/dist/latest-v20.x/docs/api/esm.html#importmetadirname)
There was a problem hiding this comment.
in here there'll always be a file:// protocol, I think? Due to the pathToFileURL on line 521
There was a problem hiding this comment.
Thats a good point, let me update it.
alesmenzel
force-pushed
the
am/add-import-meta-dirname-and-filename
branch
from
4c44b56 to
7abba69
Compare
alesmenzel
force-pushed
the
am/add-import-meta-dirname-and-filename
branch
from
b37f48d to
d04fb81
Compare
| @@ -520,6 +520,11 @@ export default class Runtime { | |||
| initializeImportMeta: (meta: JestImportMeta) => { | |||
| meta.url = pathToFileURL(modulePath).href; | |||
|
|
|||
| // @ts-expect-error Jest uses @types/node@16. Will be fixed when updated to @types/[email protected] | |||
| meta.filename = fileURLToPath(meta.url); | |||
There was a problem hiding this comment.
should we only add these if the underlying Node version supports them? That way the tests won't behave differently than at runtime if people rely on it
There was a problem hiding this comment.
I would say it should not be necessary as users on node < 20.11 won't use those variables in code.
There was a problem hiding this comment.
@SimenB Do you want to add the check? I would backport this change also to v29 when it is ready.
There was a problem hiding this comment.
Do you want to add the check?
nah, I'm fine with it. If a feature check were easy it'd be fine, but it's a bit clunky, and I don't wanna check against specific node versions.
I would backport this change also to v29 when it is ready.
We don't do backports, so there's no need for that. Thanks for offering, though!
|
New test is failing on windows. probably need a |
Right, that will be the opposite slash probably. |
| jest: expect.anything(), | ||
| url: expect.any(String), | ||
| }); | ||
| expect(import.meta.jest).toBe(jestObject); | ||
| expect( | ||
| import.meta.url.endsWith('/e2e/native-esm/__tests__/native-esm.test.js'), | ||
| ).toBe(true); | ||
| if (process.platform === 'win32') { |
There was a problem hiding this comment.
is this correct? If it's a file:// thing, it should use forward slashes. See examples in https://nodejs.org/api/url.html#urlpathtofileurlpath
There was a problem hiding this comment.
I don't have access to a windows PC, or this'd be easy to check 😅
@G-Rath would you be able to quickly check for us what import.meta.filename and import.meta.dirname returns on a windows machine?
There was a problem hiding this comment.
Testing with an arbitrary file on v21 gives me:
// file.js
import { fileURLToPath } from 'node:url';
console.log(fileURLToPath(import.meta.url));
console.log(import.meta.filename);
console.log(import.meta.url);
C:\Users\G-Rath\workspace\projects-oss\jest\file.mjs
C:\Users\G-Rath\workspace\projects-oss\jest\file.mjs
file:///C:/Users/G-Rath/workspace/projects-oss/jest/file.mjs
Let me know if that's enough or if you'd like me to try actually running this test (or other arbitrary bits of code)
There was a problem hiding this comment.
@SimenB filename and dirname are OS specific, but meta.url has URL format with forward slashes
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
Make jest compatible with Node 20.11.0 and above running ESM code.
Adds support for
Test plan
See added test case.