Skip to content

feat(cjs-to-esm): context-local variables → import.meta.*#399

Open
AugustinMauroy wants to merge 5 commits intowip/cjs-to-esmfrom
augustin/issue#348
Open

feat(cjs-to-esm): context-local variables → import.meta.*#399
AugustinMauroy wants to merge 5 commits intowip/cjs-to-esmfrom
augustin/issue#348

Conversation

@AugustinMauroy
Copy link
Member

close #348

Note

the base is feature/cjs-to-esm

@AugustinMauroy AugustinMauroy requested a review from a team March 21, 2026 09:45
@JakobJingleheimer JakobJingleheimer changed the title Augustin/issue#348 feat(cjs-to-esm): context-local variables → import.meta.* Mar 21, 2026
@AugustinMauroy AugustinMauroy added the awaiting reviewer Author has responded and needs action from the reviewer label Mar 21, 2026
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 understand this one. This looks like CJS, but it isn't transformed.

Copy link
Member Author

Choose a reason for hiding this comment

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

oh I didn't recall why I was requiring this test
maybe we can remove it

@JakobJingleheimer
Copy link
Member

Why did CI not run on this PR? O.o

@AugustinMauroy
Copy link
Member Author

Why did CI not run on this PR? O.o

not main base I think

AugustinMauroy and others added 2 commits March 21, 2026 23:08
Co-authored-by: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com>
@AugustinMauroy
Copy link
Member Author

added jsdocs !

Co-Authored-By: Bruno Rodrigues <swe@brunocroh.com>
Copy link
Member

@brunocroh brunocroh left a comment

Choose a reason for hiding this comment

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

Brilliant

@AugustinMauroy
Copy link
Member Author

Brilliant

thanks !

Copy link
Member

@JakobJingleheimer JakobJingleheimer left a comment

Choose a reason for hiding this comment

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

🙌

Comment on lines +71 to +75
if (result) {
edits.push(result.replace('import.meta.main'));
} else {
edits.push(node.replace('import.meta.main'));
}
Copy link
Member

Choose a reason for hiding this comment

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

nit: it's a little difficult to quickly see how these lines are different. I think this would both help with that and DRY things up a tiny bit

Suggested change
if (result) {
edits.push(result.replace('import.meta.main'));
} else {
edits.push(node.replace('import.meta.main'));
}
edits.push((result || node).replace('import.meta.main'));

or if result is just potentially nullish

Suggested change
if (result) {
edits.push(result.replace('import.meta.main'));
} else {
edits.push(node.replace('import.meta.main'));
}
edits.push((result ?? node).replace('import.meta.main'));

@JakobJingleheimer JakobJingleheimer added g2g No outstanding concerns, discussions, or issues. and removed awaiting reviewer Author has responded and needs action from the reviewer labels Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

g2g No outstanding concerns, discussions, or issues.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants