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

andrewbranch · 2023-06-14T16:12:37Z

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 webpack/webpack#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 microsoft/TypeScript#54102.)

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

src/index.ts

johnnyreilly · 2023-06-14T16:28:07Z

src/index.ts

+      loaderContext,
+      instance.program || instance.languageService?.getProgram() || instance.builderProgram?.getProgram()
+    ); 
+    if (impliedNodeFormat === /*ts.ModuleKind.ESNext*/ 99) {


"I got 99 problems but EcmaScript modules ain't one"

johnnyreilly · 2023-06-14T16:31:57Z

src/instances.ts

+  if (loaderOptions.transpileOnly) {
+    (transformers ??= {}).before = [
+      getSetImpliedNodeFormatTransformer(instance, loaderContext, getProgram),
+      ...(transformers?.before ?? []),


~~this reminds me, I wonder whether we should bump the target for the emitted JavaScript of ts-loader at some point. I suspect it's quite an old version.~~ "target": "es2018", - not too old

alexander-akait · 2023-06-14T16:33:05Z

src/servicesHost.ts

+
+
+  // make a (sync) resolver that follows webpack's rules
+  const resolveSync = makeResolver(loader._compiler!.options);


Do you really need this? Based on logic it is just an error - https://github.com/TypeStrong/ts-loader/blob/main/src/resolver.ts#L11

Webpack has great API for resolving, you can use (just an example of code, we should have two resolvers for cjs and esm in the real life):

const resolve = loaderContext.getResolve({ dependencyType: "typescript", conditionNames: ["types", "..."], mainFiles: ["index", "..."], extensions: [".ts", ".cts", "..."], });

And hide a lot of options for basic resolve under the hood - less options better DX, but yeah, I think it should be done in another PR, here is just about ESM

This is just moved code, not code I wrote

alexander-akait

Just some notes about code

johnnyreilly · 2023-06-14T16:36:24Z

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.

andrewbranch · 2023-06-14T16:47:23Z

...son-tests/projectReferencesNoSourceMap/expectedOutput-transpile-5.0/lib/tsconfig.tsbuildinfo

I’m not 100% sure, but I think the slight tsbuildinfo format change here is due to having a (no-op, in this case) custom transformer present. The change didn’t look meaningful so I wasn’t too worried about it.

andrewbranch · 2023-06-14T16:50:07Z

test/comparison-tests/esm_packageJsonType/src/app.ts

+import assert from "assert";
+import externalLib from "../lib/externalLib.js";
+assert.deepStrictEqual(externalLib, { default: {} });
+console.log("PASS");


I actually tried to use an execution test for this, but I think Karma or another tool in the pipeline was interfering with what I intended to run. This is a comparison test, but the output is executable in Node, and asserts the behavior I’m implementing. Without the { "type": "module" } in the package.json (or without this PR’s changes), externalLib would be {}. But here, it’s { default: {} }. ✨

alexander-akait · 2023-06-14T17:28:09Z

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 · 2023-06-14T17:30:54Z

@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 · 2023-06-15T15:58:20Z

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

johnnyreilly · 2023-06-18T12:30:43Z

Test pack migrated to 5.1 in #1616

andrewbranch · 2023-12-18T18:38:42Z

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 · 2023-12-18T20:12:38Z

Merry Christmas @andrewbranch 🎄⛄!

andrewbranch added 9 commits June 12, 2023 10:59

Add mts test

d490c50

Set module type to ESM when impliedNodeFormat is ESNext

c3fc615

Add test for package.json with type: module

4712067

Update test

394c7a7

Add expected output

3ea6e50

test script deleted part of my test

a2cbf15

Fix transpile emit

6d31cf6

Fix project references crash

379e771

Update snapshots I guess

2888b33

alexander-akait reviewed Jun 14, 2023

View reviewed changes

src/index.ts Outdated Show resolved Hide resolved

johnnyreilly reviewed Jun 14, 2023

View reviewed changes

alexander-akait reviewed Jun 14, 2023

View reviewed changes

Use resourcePath instead of module.resource

e436dd0

andrewbranch commented Jun 14, 2023

View reviewed changes

jakebailey mentioned this pull request Sep 15, 2023

Apply same ESM/CJS interop in .mts/.ts files as in .mjs/.js files webpack/webpack#17288

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

andrewbranch commented Jun 14, 2023

johnnyreilly Jun 14, 2023

johnnyreilly Jun 14, 2023 •

edited

Loading

alexander-akait Jun 14, 2023

andrewbranch Jun 14, 2023

alexander-akait left a comment

johnnyreilly commented Jun 14, 2023

andrewbranch Jun 14, 2023

andrewbranch Jun 14, 2023

alexander-akait commented Jun 14, 2023

andrewbranch commented Jun 14, 2023

alexander-akait commented Jun 15, 2023

johnnyreilly commented Jun 18, 2023

andrewbranch commented Dec 18, 2023

johnnyreilly commented Dec 18, 2023



		// make a (sync) resolver that follows webpack's rules
		const resolveSync = makeResolver(loader._compiler!.options);

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

Are you sure you want to change the base?

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

Conversation

andrewbranch commented Jun 14, 2023

johnnyreilly Jun 14, 2023

Choose a reason for hiding this comment

johnnyreilly Jun 14, 2023 • edited Loading

Choose a reason for hiding this comment

alexander-akait Jun 14, 2023

Choose a reason for hiding this comment

andrewbranch Jun 14, 2023

Choose a reason for hiding this comment

alexander-akait left a comment

Choose a reason for hiding this comment

johnnyreilly commented Jun 14, 2023

andrewbranch Jun 14, 2023

Choose a reason for hiding this comment

andrewbranch Jun 14, 2023

Choose a reason for hiding this comment

alexander-akait commented Jun 14, 2023

andrewbranch commented Jun 14, 2023

alexander-akait commented Jun 15, 2023

johnnyreilly commented Jun 18, 2023

andrewbranch commented Dec 18, 2023

johnnyreilly commented Dec 18, 2023

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

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

johnnyreilly Jun 14, 2023 •

edited

Loading