-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
feat: support import.meta.dirname and import.meta.filename #14854
Changes from 4 commits
7abba69
d04fb81
18f3b0c
a95a074
0c19357
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 |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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.
We don't do backports, so there's no need for that. Thanks for offering, though! |
||
// @ts-expect-error Jest uses @types/node@16. Will be fixed when updated to @types/[email protected] | ||
meta.dirname = path.dirname(meta.filename); | ||
|
||
let jest = this.jestObjectCaches.get(modulePath); | ||
|
||
if (!jest) { | ||
|
@@ -672,6 +677,13 @@ export default class Runtime { | |
initializeImportMeta(meta: ImportMeta) { | ||
// no `jest` here as it's not loaded in a file | ||
meta.url = specifier; | ||
|
||
if (meta.url.startsWith('file://')) { | ||
// @ts-expect-error Jest uses @types/node@16. Will be fixed when updated to @types/[email protected] | ||
meta.filename = fileURLToPath(meta.url); | ||
// @ts-expect-error Jest uses @types/node@16. Will be fixed when updated to @types/[email protected] | ||
meta.dirname = path.dirname(meta.filename); | ||
} | ||
}, | ||
}); | ||
} | ||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NOTE: Conflicting name with imported |
||
|
||
if ( | ||
await this._shouldMockModule( | ||
referencingIdentifier, | ||
path, | ||
specifierPath, | ||
this._explicitShouldMockModule, | ||
) | ||
) { | ||
return this.importMock(referencingIdentifier, path, context); | ||
return this.importMock(referencingIdentifier, specifierPath, context); | ||
} | ||
|
||
const resolved = await this._resolveModule(referencingIdentifier, path); | ||
const resolved = await this._resolveModule( | ||
referencingIdentifier, | ||
specifierPath, | ||
); | ||
|
||
if ( | ||
// json files are modules when imported in modules | ||
|
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.
is this correct? If it's a
file://
thing, it should use forward slashes. See examples in https://nodejs.org/api/url.html#urlpathtofileurlpathThere 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 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
andimport.meta.dirname
returns on a windows machine?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.
Testing with an arbitrary file on v21 gives me:
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SimenB filename and dirname are OS specific, but meta.url has URL format with forward slashes
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.
Perfect, thanks @G-Rath!