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

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

Open
andrewbranch opened this issue May 30, 2023 · 23 comments
Open

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

andrewbranch opened this issue May 30, 2023 · 23 comments
Labels

Comments

@andrewbranch
Copy link

Feature request

Webpack changes the behavior of imports of CJS files with __esModule to be more compatible with Node when the importing file extension is .mjs, or .js in scope of a package.json with "type": "module":

// @Filename: dep.js
Object.defineProperty(exports, "__esModule", { value: true });
exports.default = "default";

// @Filename: main1.js
import dep from "./dep.js";
console.log("from .js", dep);

// @Filename: main2.mjs
import dep from "./dep.js";
console.log("from .mjs", dep);
from .js default
from .mjs { [__esModule], default }

For TypeScript to offer checking behavior that reflects this, the behavior of .ts files needs to match the behavior of .js files, and the behavior of .mts files needs to match the behavior of .mjs files. However, Webpack doesn’t know what these extensions are, so they don’t get considered for Webpack’s Node-like ESM/CJS interop layer. If I add main3.mts with identical content, I get:

from .js default
from .mjs { [__esModule], default }
from .mts default

The fact that a default import behavior is different in a .mjs and .mts file makes it impossible for TypeScript to implement a module checking mode for Webpack.

What is the expected behavior?

  1. The behavior of an import in Webpack does not change when renaming the importing file extension between .ts and .js, .mts and .mjs, or .cts and .cjs.
  2. Module syntax and interop restrictions placed on .js/.mjs files based on their extension or package.json "type" are also applied to their TypeScript counterparts. For example, whenever it’s an error to require an ESM file or to use import/export syntax in a file in Webpack, that error should persist when renaming the file to its equivalent TypeScript file extension. (This is less important to us than (1).)

What is motivation or use case for adding/changing the behavior?

I’m trying to implement better support for bundlers in TypeScript, and this behavior of differentiating module interop based on file extension, but not including TS file extensions in that differentiation, is a blocker. (I can explain why if necessary, but I don’t think it’s super relevant here.) We’re tracking at microsoft/TypeScript#54102

How should this be implemented in your opinion?

I’m not a Webpack expert, but it seems like there could be advantages to letting the loader API set the module format:

  • No TypeScript-specific carve-outs need to be implemented in Webpack core
  • Other non-TS loaders that consume non-JS file extensions could adopt this behavior if desired
  • Webpack could expose the new API in a SemVer minor, while ts-loader et al. could adopt it in a SemVer major, which would probably be easier for users to adopt than waiting for a new Webpack major version

The goal for us would be that TypeScript/Webpack users get consistent behavior between .ts/.js files by default in the near future.

Are you willing to work on this yourself?

Happy to discuss solutions, and can potentially put up a PR with guidance if necessary

Additional info

I forked Tobias’s interop-test repo while researching this issue, and have changed quite a bit to show how various bundlers handle TS extensions and what they do with this issue in particular. Notably, esbuild exhibits the behavior we’re advocating for. Take a look at https://andrewbranch.github.io/interop-test/#synthesizing-default-exports-for-cjs-modules if interested.

@alexander-akait
Copy link
Member

alexander-akait commented May 30, 2023

Sorry, we can't, it was dicussed a lot of time and you can found a lot of issues about it (in 2020, 2021, 2022), people still have mix of esm and commonjs code, it will be big breaking change for webpack v5

@andrewbranch
Copy link
Author

it was dicussed a lot of time and you can found a lot of issues about it (in 2020, 2021, 2022)

This is not possible, because the TypeScript-specific file extensions I’m referring to were first created one year ago, in 2022.

Developer can setup type: "javascript/esm" on the loader level

Can you say more about this? It sounds like what I’m proposing, but I couldn’t find anything about it in the docs. https://webpack.js.org/guides/ecma-script-modules/#flagging-modules-as-esm doesn’t give much info on how a loader would interact with this.

@alexander-akait
Copy link
Member

alexander-akait commented May 30, 2023

This is not possible, because the TypeScript-specific file extensions I’m referring to were first created one year ago, in 2022.

I mean other developers ask this question (not you 😄 )

Can you say more about this? It sounds like what I’m proposing, but I couldn’t find anything about it in the docs. https://webpack.js.org/guides/ecma-script-modules/#flagging-modules-as-esm doesn’t give much info on how a loader would interact with this.

const path = require("path");

module.exports = {
	mode: "production",
	devtool: "source-map",
	entry: {
		main: "./src/entry.mjs",
	},
	module: {
		rules: [
			{
				test: /main1\.js$/,
				type: "javascript/esm",
			}
		],
	},
};

output:

from .js { default: 'default' }
from .mjs { default: 'default' }

@andrewbranch
Copy link
Author

Yeah, that doesn’t look like something we can expect every user to set up in their own ts-loader config. Also, it doesn’t seem like it allows the type for a .ts file to be set based on its package.json context. User-land config may be an interim solution, but at some point we would expect webpack + [any TypeScript loader] to implement an interop behavior that is possible to type check without additional configuration.

@alexander-akait
Copy link
Member

@andrewbranch esbuild and webpack do the same:

esbuild --bundle ./src/entry.mjs &> esbuild.js && node esbuild.js

output:

from .js default
from .mjs { default: 'default' }

webpack

webpack-cli && node ./dist/main.js

output:

from .js default
from .mjs { default: 'default' }

I think without additional options on typescript level it is impossible to solve, look at other bundlers, they have the same problem and provide option(s) to change logic (some of them are broken by default 😄 ), so developers can setup any configuration and you will not work out of the box.

Yeah, that doesn’t look like something we can expect every user to set up in their own ts-loader config. Also, it doesn’t seem like it allows the type for a .ts file to be set based on its package.json context. User-land config may be an interim solution, but at some point we would expect webpack + [any TypeScript loader] to implement an interop behavior that is possible to type check without additional configuration.

Webpack knows nothing about ts-loader (other loaders). So you can ask about it ts-loader developers. We internally discussed about changing it before release webpack v5, but when checking popular repositories at that time, it broke almost everything, now I think the situation is much better, but unfortunately still not good enough.

Technically you are right and I agree that "type": "module" should set "javascript/esm" everywhere, we can start warning about potential problems and then, in future, enable it by default (I think we have an issue about it)

@andrewbranch
Copy link
Author

Technically you are right and I agree that "type": "module" should set "javascript/esm" everywhere

This is very much not what I’m proposing. What we need is for .ts files to act like .js files and .mts files to act like .mjs files. That’s all.

I think without additional options on typescript level it is impossible to solve

I am the person trying to make additional options in TypeScript; that’s why I opened this issue 😄 Additional options are impossible for us to make for bundlers that can’t treat TS extensions the same as JS extensions.

esbuild and webpack do the same

Please see https://andrewbranch.github.io/interop-test/#synthesizing-default-exports-for-cjs-modules again—esbuild differs between .mjs and .js files like Webpack; I think this is good 👍. But esbuild also treats .mts and .mjs files the same (good 👍), where Webpack treats them differently (bad 👎). That’s why esbuild gets the 💙 in the table and Webpack does not. My goal here is to get Webpack a 💙.

@alexander-akait
Copy link
Member

Please see https://andrewbranch.github.io/interop-test/#synthesizing-default-exports-for-cjs-modules again—esbuild differs between .mjs and .js files like Webpack; I think this is good +1. But esbuild also treats .mts and .mjs files the same (good +1), where Webpack treats them differently (bad -1). That’s why esbuild gets the blue_heart in the table and Webpack does not. My goal here is to get Webpack a blue_heart.

Hm, can you provide an example with webpack configuration and code? I may be confused because it should work identically, if not, I want to say it is a bug on ts-loader side, but I want to investigate

@alexander-akait
Copy link
Member

alexander-akait commented May 30, 2023

And what output do you expect, just to clarify

@andrewbranch
Copy link
Author

https://github.com/andrewbranch/example-webpack-ts-extensions

I would be thrilled if it were a ts-loader bug, but I didn’t think loaders had a way of controlling this.

@alexander-akait
Copy link
Member

I would be thrilled if it were a ts-loader bug, but I didn’t think loaders had a way of controlling this.

Thank you, I will look soon, webpack doesn't know when source code is stricky ESM, so I am afraid we can't do this without loader API, mts and cts, and even ts are unknown extensions 😄

But I will provide more feedback later

@alexander-akait
Copy link
Member

alexander-akait commented Jun 4, 2023

@andrewbranch I looked at code and yeah, type: "javascript/esm" solves the problem, technically it can be fixed here https://github.com/webpack/webpack/blob/main/lib/config/defaults.js#L633, just change it to test: /\.m(js|ts)$/i,, but:

  • webpack doesn't know non standard extensions and I think loader should do this things
  • Is it possible to generate commonjs/other in mts? Is it always ESM? I don't know some small details so need to clarify
  • Another very simple solution is put loaderContext._module.type = "javascript/esm"; in ts-loader, here https://github.com/TypeStrong/ts-loader/blob/main/src/index.ts#L136 (it will not work for happypack, but it is not popular usage, for happypack users the solution is set type: "javascript/esm" manually)

I think the last solution is better because only ts-loader and typescript know when the source code is ESM (I think typescript can have API to get it, at the least there is tsconfig.json to undestand it).

Also I can implement public API to change type of a module (but it will require some dicussions some loaders may start to abuse it and this is not very good, although it is still possible) to avoid using _module.

And yes - updating ts-loader docs for ESM will be great too.

Also my concern is that if we add cts and mts, other tools can start to ask do add them extensions too, this will make many settings inflexible and may create a situation when, with a new update of a tool we will need to change something, and this may be a breaking change(s).

@andrewbranch
Copy link
Author

I agree that a loader would be a good place to set the module kind. It’s great that there’s a (private API?) way to do it already!

Is it possible to generate commonjs/other in mts?

Only under unsupported combinations of tsconfig options. I’m looking into making those combinations illegal.

Also I can implement public API to change type of a module (but it will require some dicussions some loaders may start to abuse it and this is not very good, although it is still possible) to avoid using _module.

I think this would be great. Please keep me posted on these discussions. Thanks for looking into this!

@alexander-akait
Copy link
Member

Only under unsupported combinations of tsconfig options. I’m looking into making those combinations illegal.

Yeah, it will be good

I think this would be great. Please keep me posted on these discussions. Thanks for looking into this!

Theoretically, we can fix it right now 😄 Also if we will use loaderContext._module.type = "javascript/esm"; we will support old webpack versions too, then we can implement public API to change it, so almost every webpack versions (for 5) will work with it

@andrewbranch
Copy link
Author

Yeah. I’ll plan to open a PR on ts-loader (maybe some others?), but a public API would be good, just so I can be sure the functionality will be available in future Webpack versions.

@alexander-akait
Copy link
Member

@andrewbranch I agree, maybe we should just add test cases to avoid regression in future, I will do it when we shipped it to ts-loader, so we it will be a guarantee for us (not 100%), but better than nothing

@webpack-bot
Copy link
Contributor

This issue had no activity for at least three months.

It's subject to automatic issue closing if there is no activity in the next 15 days.

@jakebailey
Copy link
Contributor

TypeStrong/ts-loader#1614 seems to be stalled in the ts-loader side, so I'm going to be "that guy" to keep the thread alive for a little longer until the official public API for this makes its way into webpack 😄

@andrewbranch
Copy link
Author

The root of the stalling is actually my part on TypeScript itself 😅

@webpack-bot
Copy link
Contributor

Issue was closed because of inactivity.

If you think this is still a valid issue, please file a new issue with additional information.

@alexander-akait
Copy link
Member

Still valid for ts-loader

@andrewbranch
Copy link
Author

Yes, a small update: I’m finally unblocked to work on the TS side of this issue and hope it will land in 5.5. Thanks for the patience!

@alexander-akait
Copy link
Member

@andrewbranch Friendly ping, any news? Do you need a help?

@andrewbranch
Copy link
Author

This is a prerequisite: microsoft/TypeScript#57896

If this lands within a week or two, I probably have time to do the second part in TS in time to ship in 5.5.

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

No branches or pull requests

4 participants