-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Dual ESM/CJS emit with tsc #54593
Comments
This isn't an approach I would recommend. At one point I was looking into using "stub" package.json files to set For example, consider a structure like this:
and a root package.json like this: {
"type": "commonjs",
"exports": {
".": {
"require": "./dist/cjs/index.js",
"import": "./dist/esm/index.js"
}
},
"imports": {
"#lib": {
"types": "./lib/compat.d.ts",
"node": "./lib/compat.node.js",
"browser": "./lib/compat.browser.js"
}
}
} You would define your import * from "#compat";
... At compile time it seems like this will work, because there is no stub package.json file in the /src directory. As such, resolution walks up to the package.json in the root and can successfully resolve the I ran into this in https://github.com/esfx/esfx/tree/main/packages/equatable, which forced me to abandon that approach. |
Instead, the approach I took was to just build .cjs outputs to /dist/cjs and then transform them to .mjs (including extension renaming) under /dist/esm. I know we've been reticent to rewrite imports during emit, but I think building a |
Central to my proposal is considering not just the package.json files that already exist on disk, but the ones that we know are going to exist due to emit from this feature. Accurate compile time format detection and module resolution is table stakes and is relatively easily achievable. However, I didn’t think about the fact that this would block the root-level |
As someone who would want to use this feature, this would make it completely unusable for me. Honestly, I'd love to see package.json add some mechanism of specifying the default |
That’s exactly what @DanielRosenwasser said 👀 |
I do wonder if |
I'm not sure if there are other implications to using stub package.json files we would have to consider. I would expect that extension mangling would be far easier and far more reliable than trying to rewrite There are a number of other considerations as well. Some packages expect to walk up to find |
Doesn't this open the exact same can of worms as rewriting paths in import specifiers? |
No, it opens a can of worms which is a strict subset of the can of worms you mentioned. |
Shameless plug: I wrote hackbg/ubik to solve this exact scenario (source since the link seems to be missing from NPM). It works well enough in my case (publishing CJS, ESM and DTS side by side from the same TS source) -- though admittedly it's still slightly rough around the edges. Give it a try if you want, feedback appreciated 😉 |
If you're tired of waiting for this, I would recommend @knighted/duel (of course I would). |
@rbuckton gets it. Except, make sure only one package.json file is required, the two package.json thing is not necessary. Or just opt out of supporting the two file new extensions. |
(#8) BREAKING CHANGE: deprecates --target-extension
RE: Comment
This risks TypeScript emitting incorrect output code because TypeScript infers If it is set to The approach I have taken is similar but requires adding another
and a root package.json like this: {
"type": "commonjs",
"exports": {
".": {
"require": "./dist/cjs/index.js",
"import": "./dist/esm/index.js"
}
} With this configuration and compiler options You also cannot use |
What you're describing is exactly what tshy does, linked in your other issue: #55925 (comment) If you're not type checking twice, there's no guarantee that things will work the way you expect. |
tshy is a cool solution, very Parcel-like - but I would prefer to limit additional tooling as much as possible. Listing the army of tools needed for just a basic production-ready TypeScript project already sounds like the start of an incantation 😆. I guess I could write a build script that rewrites the Perhaps adding a compiler option like |
Considering making the format detection customizable as part of #55221 (e.g. input-relative package.json, output-relative package.json, force ESM, force CommonJS) |
I think we are really making things overcomplicated. What about just bring the compatibility back, and a suppressible warning and we are good as the old days. And then we can discuss the refined solution with ease. |
I just tried again to fix the types for all the PostCSS plugins under Our requirements :
Our users requirements :
Our packages on npm contain both However it is impractical to also have correct types for all our users and all their configs. The only way I see that we could do this is to prevent TypeScript from emitting types and to then manually write Hurdles/issues which I expect to be solved by the TypeScript team, not because I take it for granted that others do this work, but because I suspect that they are best positioned to solve this.
For me these two are linked. I don't want to randomly change our I really hope that something can be done here because this is holding back so much progress. If dual emit did work, more packages could easily set this up correctly. That would allow more end users to migrate to |
In the end we decided to not do dual typing even though we are dual publishing as commonjs and es modules. There is just no way to get this working in our case without making the whole substantially worse and harder to maintain. We are now only providing types for es modules. |
Since there is no real analogue to top level await in CommonJS, you might as well just wait until |
If I'm only using tsc for emitting types, is there any reason to run TSC twice? Or can I just reuse the esm types for the cjs output? Is there any scenario where the esm and cjs types would differ? |
Surely you're using tsc to typecheck, not just emit, so running twice would still be the only way to check two times that your code functions in both modes. Some dep may have totally different types depending on whether it was required versus imported. |
Thanks for your reply 🙏
Yes, you're right 🙂 What I meant was I was using it with Some of our packages I can get to build for CJS pretty much as they are by just (temporarily) changing to (We're building ESM to |
FWIW this is pretty much exactly what https://www.npmjs.com/package/tshy automates. |
#54546 explored one approach of enabling dual ESM/CJS emit for packages with tsc alone. The idea was that instead of determining the module format of .ts files by looking for package.json files in its ancestor directories, we would look for them starting at the directory where that file’s
.js
output would be emitted (a subdirectory ofoutDir
). That way, two tsconfig files could point to two different output directories, each pre-seeded with a package.json files that set the module format for the output generated by that tsconfig.This approach has two main downsides:
"type"
. This can be a problem if a project compiles with tsc for publishing, but runs input .ts files directly during development (which is a scenario I think we should make a habit of considering). Ideally, we want a solution that ensures the module format of the output agrees with that of the input, unless the output format is being intentionally changed by config (presumably for purposes of dual emitting).I think both of these can be solved by doing two things:
{ "type": "module" }
or{ "type": "commonjs" }
(or blank, whatever) into theoutDir
, and use this config setting (instead of what’s already present in theoutDir
) to determine the module format of input files at a higher priority than any package.json files in a directory higher thanoutDir
. This solves problem (1) above—no need to pre-seed or commit files in your build directory.rootDir
that affects the computed module format of a file is seen, emit that package.json (or a stub of it with just"type"
?) into the corresponding subfolder withinoutDir
. This solves (2), and actually solves an issue that exists today, where tsc output can be invalid for Node without manually copying a package.json that occurs insiderootDir
. (@rbuckton mentioned this in team chat one time, but it didn’t get much discussion.)Emitting package.json files would be new territory for us, but I think it’s worth it for the problems it solves.
The text was updated successfully, but these errors were encountered: