TypeStrong / ts-loader

TypeScript loader for webpack
https://johnnyreilly.com/ts-loader-goes-webpack-5
MIT License
3.44k stars 429 forks source link

Set webpack module type to `"javascript/esm"` when TS `impliedNodeFormat` is `ESNext` #1614

Open andrewbranch opened 1 year ago

andrewbranch commented 1 year ago

Or in other words, make Webpack give the same special treatment to .mts files as it does .mjs files. (Same for .ts in scope of a package.json "type": "module".) In short, Webpack changes its ESM/CJS module interop strategy to match Node’s when it sees these file extensions. In order for TypeScript to accurately type check this behavior, it needs to be consistent between JS and TS files. See https://github.com/webpack/webpack/issues/17288 and https://andrewbranch.github.io/interop-test/#synthesizing-default-exports-for-cjs-modules for more of an explanation. (This PR will give Webpack + ts-loader + "module": "nodenext" a 💙🌟 in the linked table.)

This is technically a breaking change, but it only affects users who are using node16/nodenext for their moduleResolution, which I suspect is rare in combination with Webpack. In the near future, I’m planning to make a new module or other TypeScript option that enables this Node-like interop behavior in .mts/.mjs files, but can be used with moduleResolution: "bundler". (This is tracked by https://github.com/microsoft/TypeScript/issues/54102.)

I think this should stay open until https://github.com/microsoft/TypeScript/issues/54102 has a clear resolution; I just wanted to make sure it was doable before moving on. Thanks @alexander-akait for the pointers in https://github.com/webpack/webpack/issues/17288.

johnnyreilly commented 1 year ago

This looks generally good - and it looks like @alexander-akait is providing some excellent feedback. I realise I should sort out upgrading the ts-loader test pack to 5.1 as well; will sort that in the next week sometime.

alexander-akait commented 1 year ago

just idea - maybe when developer has javascript/dynamic (i.e. define type: "javascript/dynamic") and you want to say javascript/esm we should print a warking, for example like "Typescript emits ES modules, but you handle it as regular Javascript, please remove type: "javascript/dynamic" or set it to javascript/esm" (we have loaderContext.emitWarning(new Error(message)) API for this).

andrewbranch commented 1 year ago

@alexander-akait if someone defines the type in their loader config, which value gets used? The one I set in this PR or the loader config value?

alexander-akait commented 1 year ago

@andrewbranch Your value, you override it, that is why I think we need to emit a warning to catch some weird configurations/options

johnnyreilly commented 1 year ago

Test pack migrated to 5.1 in https://github.com/TypeStrong/ts-loader/pull/1616

andrewbranch commented 6 months ago

Just wanted to drop in and say this is still on my radar; just figuring out how to expose the related config in TypeScript itself is proving difficult.

johnnyreilly commented 6 months ago

Merry Christmas @andrewbranch 🎄⛄!