Skip to content
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

Global CommonJS data files in Node 23 do not load their data #3518

Closed
vrugtehagel opened this issue Oct 29, 2024 · 3 comments · Fixed by #3519
Closed

Global CommonJS data files in Node 23 do not load their data #3518

vrugtehagel opened this issue Oct 29, 2024 · 3 comments · Fixed by #3519
Assignees
Labels

Comments

@vrugtehagel
Copy link
Contributor

vrugtehagel commented Oct 29, 2024

Eleventy

3.0.0

Describe the bug

When using CommonJS syntax in Node 23, global data files are not loaded properly and results in the data being unavailable throughout the build.

This is because Module objects for modules using module.exports now contain two keys, 'default' and 'module.exports', and Eleventy specifically checks if there is only one before accessing the default property. Then it goes on to make sure the keys from 'default' and those in the top level are the same, which fails on the 'module.exports' key.

@zachleat
Copy link
Member

zachleat commented Dec 6, 2024

Confirmed this one as a bug.

image

My issue here is that Node 23 is an odd version of Node—is there any indication that this change is going to ship for an even version?

@zachleat zachleat self-assigned this Dec 6, 2024
@vrugtehagel
Copy link
Contributor Author

Good point @zachleat! I actually don't know if it's here to stay, but it does kind of look like it from Node's ESM docs (see also the commit):

These CommonJS namespace objects also provide the default export as a 'module.exports' named export, in order to unambiguously indicate that their representation in CommonJS uses this value, and not the namespace value. This mirrors the semantics of the handling of the 'module.exports' export name in require(esm) interop support.

This not so much specifies whether it is a temporary change or not, but it does seem like Node's attempting to document it properly and has reasons for its existance (which, I'll be honest, I don't quite follow, but they're there).

Either way, given the fix for it seems rather unintrusive, I think it'd just be a nice-to-have for those on 23 (and potentially beyond) without bothering anyone on Node <23 (I highly doubt anyone's explicitly exporting a 'module.exports' key).

If you really are worried about this breaking things for users on lower Node versions, we can introduce an extra check against the Node version to make it a Node 23-only change.

@zachleat
Copy link
Member

I agree, thanks for talking it out! We’ll ship this with v3.0.1-alpha.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants