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

Consider unstable flag to make cjs files be treated as esm #22818

Closed
dsherret opened this issue Mar 9, 2024 · 6 comments · Fixed by #22945
Closed

Consider unstable flag to make cjs files be treated as esm #22818

dsherret opened this issue Mar 9, 2024 · 6 comments · Fixed by #22945
Labels
suggestion suggestions for new features (yet to be agreed)

Comments

@dsherret
Copy link
Member

dsherret commented Mar 9, 2024

It would be useful to have an unstable flag that allows people to import CJS files that contain imports/exports similar to Node's --experimental-detect-module.

For example: #22811

@dsherret dsherret added this to the 1.42 milestone Mar 9, 2024
@dsherret dsherret added suggestion suggestions for new features (yet to be agreed) and removed enhancement labels Mar 9, 2024
@mash-graz
Copy link
Contributor

mash-graz commented Mar 11, 2024

There are already so much "unstable" flags available, that I would try to avoid another one, whenever possible.

But it would be very appreciable, if we could have a central detection mechanism -- including something similar to nodejs/node#50096 resp. its containsModuleSyntax check -- to prevent unnecessary duplication of this kind of decision-making.

Right now deno is handling this kind of detection/decision task at a few different places.

That's why I'm fighting in #22840 some node:worker_thread import related troubles, which look surprisingly similar to #22811, which you solved just recently.

Please could you take a scrutinizing look at my draft PR (#22841) to solve this issue in this other place as well. Nevertheless, a more general solution for the file type detection and more reliable unit tests for its proper application would indeed make a lot of sense in the long run.

@rracariu
Copy link

I would also prefer this to be handled automatically, same as Bun does, and for sure Node would also do this as soon they stabilize their experimental flag described here.

@dsherret
Copy link
Member Author

dsherret commented Mar 11, 2024

Once Node does that then we'll also change the behaviour to align with Node, but doing non-standard Node behaviour for Node compat makes the ESM/CJS situation in the Node ecosystem worse and not better (encourages people shipping and writing code that doesn't work with Node's resolution). I understand it's convenient for apps to have this behaviour though, which is why this non-standard Node behaviour will most likely be behind a flag.

@mash-graz
Copy link
Contributor

mash-graz commented Mar 11, 2024

In this particular case we just have to figure out, which kind of import format should be used for a particular file. This can be simply decided by 1. the suffix, 2. enclosing package.json metadata or 3. found ESM/CJS specific keywords within the file.

We just have to figure out one question: how to handle the given files in the most appropriate manner?

If none of this criteria fits, we can still fall back to some very old fashioned defaults...

A perfect emulation of all the faults, inflexibility and historic burden of node shouldn't be the goal. Concerning ESM support in particular, we should really try to handle these tasks simply better than in node and also strive for unchanged legacy code utilization in modern runtimes --i.e. the other side of compatibility and easy reuse -- wherever possible.

@dsherret
Copy link
Member Author

encourages people shipping and writing code that doesn't work with Node's resolution

Upon further reflection, what I said in my previous comment isn't as relevant to Deno because Deno already treats everything outside node_modules as an ES module, so someone could already develop an npm package without properly setting up their package.json for distribution for Node.

I opened #22945 that implements this behaviour without a flag.

@mash-graz
Copy link
Contributor

I'm in the processing of writing a more extensive answer about this topic in #17650.

We have very likely slightly different opinions on this topic, but I really like the way how you tireless advocate for improvement and practical change! :)

dsherret added a commit that referenced this issue Mar 21, 2024
Changes the behaviour in Deno to just always load ES modules in npm
packages even if they're defined as CJS.

Closes #22818
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants