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

ESBuild resolves modules by using the TypeScript moduleSuffixes per package instead of per bundle #2395

Closed
sverrejoh opened this issue Jul 18, 2022 · 6 comments

Comments

@sverrejoh
Copy link

sverrejoh commented Jul 18, 2022

The moduleSuffixes feature of TypeScript is now being used for module resolution in ESBuild, but for a bundle this should be set once per whole bundle, and not locally per package.

React Native has Platform Specific Extensions, which is used by its Metro Bundler to allow JavaScript modules to be forked per platform. So for example a Button module could have a Button.ios.jsx implementation for iOS and a Button.android.jsx for Android. The right fork (for example ios) is selected per bundle, so you choose the current platform for your bundle globally.

In TypeScript this is now supported via the moduleSuffixes configuration. You can then set the moduleSuffixes in a shared package's tsconfig.json, and TypeScript will then type-check that package using the platform extensions in the order of the moduleSuffixes list. This is though not shared outside that package. The order is only used as one arbitrary way to test a set of resolutions. If you want to type check for multiple platforms you would have to have multiple configuration files with its own moduleSuffixes list and run TypeScript once per the platform you want to test.

The problem is that ESBuild is using the per package moduleSuffixes when resolving module paths for the bundle. The correct way would be to ignore moduleSuffixes for bundling, and rather let the user set the resolution order with the per bundle ESBuild resolveExtensions setting. For example resolveExtensions: [ ".ios.tsx", ".tsx", ".ios", ".ts", ".js", ".jsx" ],.

I'm not sure I can see a use for ESBuild to support moduleSuffixes at all, but at least it should be configurable as the usual way to set the resolution is once for all the code in your bundle.

@evanw
Copy link
Owner

evanw commented Jul 18, 2022

You can override esbuild's choice of tsconfig.json with your own if you want all code in the bundle to use a specific one: https://esbuild.github.io/api/#tsconfig

@sverrejoh
Copy link
Author

Right. Thanks for the pointer! That kind of works, but I imagine I would like to override only moduleSuffixes but let esbuild infer the rest of the local tsconfig.json file.

What is the use case for bundling files in a package using the moduleSuffixes config in its tsconfig.json? The TypeScript moduleSuffixes feature is general, but it's using React Native platform forking as an example, and with React Native Platform forking it doesn't make sense to have different parts of your bundle resolve to different platforms.

@evanw
Copy link
Owner

evanw commented Jul 18, 2022

The compilation model esbuild currently uses is designed to replace the workflow where you run tsc to convert TypeScript into JavaScript and then run a bundle to bundle the JavaScript. With esbuild, all of this happens in a single step for better performance but it's supposed to still conceptually behave similar to the old workflow. This is why esbuild supports multiple tsconfig.json files: because each folder with a tsconfig.json file in it is conceptually a thing that you would traditionally run tsc in for each one before bundling. Or you can explicitly pass a single tsconfig.json file to esbuild to override all others, which is conceptually similar to running tsc once over everything. But I don't think it makes sense to take some tsconfig.json settings files from some files and others from other files since that's not something that tsc itself does.

TypeScript's documentation says this:

This feature can be useful for React Native projects where each target platform can use a separate tsconfig.json with differing moduleSuffixes.

It looks like the TypeScript team is expecting you to explicitly pass a platform-specific tsconfig.json file when building code that uses moduleSuffixes. You can also do this with esbuild (as described above).

@sverrejoh
Copy link
Author

I love that esbuild works that way, and it's amazing that one tool can deal with all the tiny details of all the tools it's replacing, but moduleSuffixes doesn't affect any of the output from running tsc. It helps the type checker to resolve the modules to check against, but the JavaScript output isn't affected by it. It'll output JavaScript that contains the same import statements as before, and those can be handled by esbuild in the right way by adding your platform extensions to the resolveExtensions bundle option — like in my example above. In the same way as the React Native Metro bundler does it, or Webpack which is also possible.

Imagine a shared npm package that implements a button with separate implementations for each platform. The developer sets up a default tsconfig.json with moduleSuffixes set to say [".ios", ".native", ""] which is the defaults when you bundle for iOS, but also has a tsconfig.android.json file with [".android", ".native", ""], which can be used to typecheck the .android files. Still, the React Native Metro bundler will resolve the JavaScript from that package using the platform extension given by the selected platform. You create one bundle for ios and one bundle for android. It will never make sense to resolve half your app to .ios and half to .android.

I don't think you need to deal with moduleSuffixes at all. esbuild doesn't type check, so moduleSuffixes isn't needed.

@evanw
Copy link
Owner

evanw commented Jul 18, 2022

But by that argument, esbuild shouldn't respect paths either, since that's also a path transformation that tsc doesn't apply. However, esbuild does currently apply paths mappings from tsconfig.json files, and from what I understand many people make use of this feature.

The rationale for paths in TypeScript (as far as I know) is to have a way to encode path resolution rules from your runtime (or bundler) into TypeScript so it can check types correctly. But rather than esbuild inventing a whole new way to specify these path remappings that's equivalent to TypeScript's paths feature, and then having to specify this remapping twice, esbuild just reuses what TypeScript has instead.

To be clear: I don't personally care about this either way. I don't use anything in Facebook's ecosystem myself. I'm just trying to figure out a consistent set of rules for how esbuild is put together. It sounds like you're saying that esbuild should not apply path remappings from tsconfig.json just like TypeScript, and then instead have a separate parallel set of esbuild-specific APIs for doing the same thing. That could have been an approach that esbuild took earlier on, but people have now come to rely on what esbuild currently does, so changing it would likely be painful.

I could potentially remove support for moduleSuffixes as you suggest and just be inconsistent about this, since it could be that TypeScript's moduleSuffixes feature is essentially specific to React Native instead of being a general-purpose feature. That would have to happen in a breaking change release though, so I'm going to mark this issue as breaking.

I also wonder what other tools do here. I could easily see Webpack doing this (having their own way of specifying path remappings). I haven't looked into this myself though.

@evanw evanw added the breaking label Jul 18, 2022
@sverrejoh
Copy link
Author

moduleSuffixes was created to be able to support Facebook's React Native, but it is implemented as a general feature, which I agree makes this less clear. In a React Native bundle it is essential that all modules in the bundle are resolved using the same platform. It has to be set to the same for all the code, and it doesn't make sense to resolve it differently per package. But if you want to use moduleSuffixes as a way to support any other kind of resolution that isn't affected by the bundle target or other global context, then esbuilds behaviour makes a lot of sense.

The path mapping is a local per package mapping, and doesn't have any of the problems that Platform Forking has where the canonical React Native bundler does the final global platform resolution.

To support React Native style platform forking in Webpack you would use the resolve.extensions list. The React standard Create React App project template actually supports the .web extension to be compatible with React Native Web. This is again global per bundle, and works in the same way as esbuild's resolveExtensions.

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

No branches or pull requests

2 participants