NotificationsYou must be signed in to change notification settings
Fork6.6k
Star45.2k

feat: support import.meta.dirname and import.meta.filename#14854

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

SimenB merged 5 commits intojestjs:mainfromalesmenzel:am/add-import-meta-dirname-and-filename

Jan 16, 2024

Merged

feat: support import.meta.dirname and import.meta.filename#14854

SimenB merged 5 commits intojestjs:mainfromalesmenzel:am/add-import-meta-dirname-and-filename

Jan 16, 2024

Conversation

Copy link

Contributor

alesmenzel commentedJan 15, 2024

Summary

Make jest compatible withNode 20.11.0 and above running ESM code.
Adds support for

import.meta.filenameimport.meta.dirname

Test plan

See added test case.

Copy link

linux-foundation-easyclabot commentedJan 15, 2024•
edited
Loading

The committers listed above are authorized under a signed CLA.

✅ login: alesmenzel / name: Aleš Menzel (7abba69,d04fb81,18f3b0c,a95a074)
✅ login: SimenB / name: Simen Bekkhus (0c19357)

Copy link

netlifybot commentedJan 15, 2024•
edited
Loading

✅ Deploy Preview forjestjs ready!

Builtwithout sensitive environment variables

Name	Link
🔨 Latest commit	`0c19357`
🔍 Latest deploy log	https://app.netlify.com/sites/jestjs/deploys/65a709227cbdac0008f3fff8
😎 Deploy Preview	https://deploy-preview-14854--jestjs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to yourNetlify site configuration.

alesmenzel commented

Jan 15, 2024

View reviewed changes

packages/jest-runtime/src/index.ts

		}

		const[path,query]=specifier.split('?');
		const[specifierPath,query]=specifier.split('?');

Copy link

ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

NOTE: Conflicting name with importednode:path.

alesmenzel commented

Jan 15, 2024

View reviewed changes

packages/jest-runtime/src/index.ts Outdated

		initializeImportMeta:(meta:JestImportMeta)=>{
		meta.url=pathToFileURL(modulePath).href;

		if(meta.url.startsWith('file://')){

Copy link

ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

NOTE: The check here is because of the CAVEAT mention in nodejs docs (seehttps://nodejs.org/dist/latest-v20.x/docs/api/esm.html#importmetadirname)

Copy link

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

in here there'll always be afile:// protocol, I think? Due to thepathToFileURL on line 521

Copy link

ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thats a good point, let me update it.

feat: support import.meta.dirname and import.meta.filename

7abba69

alesmenzel force-pushed theam/add-import-meta-dirname-and-filename branch from4c44b56 to7abba69Compare

January 15, 2024 11:17

update tests

d04fb81

alesmenzel force-pushed theam/add-import-meta-dirname-and-filename branch fromb37f48d tod04fb81Compare

January 15, 2024 14:53

remove unnecessary file:// check

18f3b0c

SimenB reviewed

Jan 15, 2024

View reviewed changes

packages/jest-runtime/src/index.ts

		meta.url=pathToFileURL(modulePath).href;

		//@ts-expect-error Jest uses @types/node@16. Will be fixed when updated to @types/node@20.11.0
		meta.filename=fileURLToPath(meta.url);

Copy link

Member

SimenBJan 15, 2024•
edited
Loading

Choose a reason for hiding this comment

The 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

Copy link

ContributorAuthor

Choose a reason for hiding this comment

The 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.

Copy link

ContributorAuthor

Choose a reason for hiding this comment

The 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.

Copy link

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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!

Copy link

Member

SimenB commentedJan 15, 2024

New test is failing on windows. probably need aslash or something. We should figure out what the value is on windows platforms, tho

Copy link

ContributorAuthor

alesmenzel commentedJan 15, 2024

New test is failing on windows. probably need aslash or something. We should figure out what the value is on windows platforms, tho

Right, that will be the opposite slash probably.

update test for windows paths

a95a074

SimenB reviewed

Jan 16, 2024

View reviewed changes

e2e/native-esm/__tests__/native-esm.test.js

		expect(
		import.meta.url.endsWith('/e2e/native-esm/__tests__/native-esm.test.js'),
		).toBe(true);
		if(process.platform==='win32'){

Copy link

Member

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 afile:// thing, it should use forward slashes. See examples inhttps://nodejs.org/api/url.html#urlpathtofileurlpath

Copy link

Member

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 whatimport.meta.filename andimport.meta.dirname returns on a windows machine?

Copy link

Contributor

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:

// file.jsimport { 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.mjsC:\Users\G-Rath\workspace\projects-oss\jest\file.mjsfile:///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)

Copy link

ContributorAuthor

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

Copy link

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Perfect, thanks@G-Rath!

Update CHANGELOG.md

0c19357

SimenB approved these changes

Jan 16, 2024

View reviewed changes

Copy link

Member

SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

thanks!

SimenB merged commitbdb86aa intojestjs:main

Jan 16, 2024

Copy link

github-actionsbot commentedFeb 16, 2024

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.
Please note this issue tracker is not a help forum. We recommend usingStackOverflow or ourdiscord channel for questions.

github-actionsbot locked asresolvedand limited conversation to collaborators

Feb 16, 2024

Copy link

Member

SimenB commentedFeb 20, 2024

https://github.com/jestjs/jest/releases/tag/v30.0.0-alpha.3

Labels

None yet

3 participants

Movatterモバイル変換

Uh oh!

feat: support import.meta.dirname and import.meta.filename#14854

feat: support import.meta.dirname and import.meta.filename#14854

Uh oh!

Conversation

alesmenzel commentedJan 15, 2024

Summary

Test plan

Uh oh!

linux-foundation-easyclabot commentedJan 15, 2024• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

netlifybot commentedJan 15, 2024• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

✅ Deploy Preview forjestjs ready!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SimenBJan 15, 2024• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SimenB commentedJan 15, 2024

Uh oh!

alesmenzel commentedJan 15, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SimenB left a comment

Choose a reason for hiding this comment

Uh oh!

github-actionsbot commentedFeb 16, 2024

Uh oh!

SimenB commentedFeb 20, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

linux-foundation-easyclabot commentedJan 15, 2024•
edited
Loading

netlifybot commentedJan 15, 2024•
edited
Loading

SimenBJan 15, 2024•
edited
Loading