Skip to content

Implement import.meta.dirname/filename in NMR - #6954

Merged
jasnell merged 1 commit into
mainfrom
jasnell/nmr-import-meta-dirname
Aug 10, 2026
Merged

Implement import.meta.dirname/filename in NMR#6954
jasnell merged 1 commit into
mainfrom
jasnell/nmr-import-meta-dirname

Conversation

@jasnell

@jasnell jasnell commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Add import.meta.dirname and import.meta.filename to new module registry import.meta support

@jasnell
jasnell requested review from a team as code owners August 10, 2026 15:53
Comment thread src/workerd/jsg/modules-new.c++ Outdated
Comment thread src/workerd/api/tests/new-module-registry-test.js
@ask-bonk

ask-bonk Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

I'm Bonk, and I've done a quick review of your PR.

This PR adds import.meta.dirname and import.meta.filename (WinterCG path helpers) for file: URL modules in the new module registry.

I posted 2 inline comments:

  1. The manual reverse loop for finding the last / can be replaced with pathname.findLast('/'), which is already used in the same file (url.c++:374) and is more idiomatic. Suggestion provided.

  2. The importMetaPathHelpers JS test claims to verify that non-file URL modules lack filename/dirname, but never actually asserts this — it only checks mod.default. Suggestion provided to actually export and assert those properties.

github run

Add `import.meta.dirname` and `import.meta.filename` to
new module registry `import.meta` support
@jasnell
jasnell force-pushed the jasnell/nmr-import-meta-dirname branch from a5260b8 to 4767eee Compare August 10, 2026 16:10
@jasnell

jasnell commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator Author

/ask-bonk update your review

@irvinebroque

Copy link
Copy Markdown
Collaborator

thank you!

@jasnell
jasnell merged commit db7ba52 into main Aug 10, 2026
33 of 37 checks passed
@jasnell
jasnell deleted the jasnell/nmr-import-meta-dirname branch August 10, 2026 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants