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

--moduleResolution bundler (formerly known as hybrid) #51669

Merged
merged 65 commits into from Dec 13, 2022

Conversation

andrewbranch
Copy link
Member

@andrewbranch andrewbranch commented Nov 28, 2022

This PR introduces a new moduleResolution setting value called hybrid bundler (see #51714), designed primarily for bundlers and runtimes that include a range of Node-like resolution features and ESM syntax, but do not enforce the strict resolution rules that accompany ES modules in Node or in the browser. Special consideration has also been given for bundlers and runtimes that understand TypeScript natively and do not require compilation to JavaScript by tsc before consumption. Additionally, resolution of package.json exports and imports can be enabled/disabled/customized in configuration options. This should allow users of different bundlers and runtimes with slight variations in resolution features to customize TypeScript’s resolution settings under bundler as appropriate.

Who should use this mode?

  • ✅ Application authors who use a bundler on their TS or JS files before a runtime consumes that bundle
  • ✅ Application authors who run in Bun
  • ✅ Library authors who use a bundler to deploy a UMD bundle
  • ⚠️ Library authors who use a tool like Rollup to deploy multiple builds in different module formats—defer to advice from your build tool
  • 🚫 Anyone intending to produce modules with tsc that will run in Node or the browser without further bundling or processing
  • 🚫 Anyone intending to produce modules with tsc that will run in Deno without further bundling or processing

Comparison with existing module resolution settings

classic node node16 bundler
node_modules packages
extensionless CJS only
directory index CJS only
*.ts imports
package.json exports
exports conditions always node, types;
import from ESM,
require from CJS;
custom additions
always types, import;
custom additions

Module syntax restrictions

--moduleResolution bundler does not support resolution of require calls. In TypeScript files, this means the import mod = require("foo") syntax is forbidden; in JavaScript files, require calls are not errors but only ever return the type any (or whatever an ambient declaration of a global require function is declared to return).

New compiler options

  • allowImportingTsExtensions: Allow imports to include TypeScript file extensions. Requires '--moduleResolution bundler' and either '--noEmit' or '--emitDeclarationOnly' to be set.
  • resolvePackageJsonExports: Use the package.json 'exports' field when resolving package imports. Enabled by default in node16, nodenext, and bundler.
  • resolvePackageJsonImports: Use the package.json 'imports' field when resolving imports. Enabled by default in node16, nodenext, and bundler.
  • customConditions: Conditions to set in addition to the resolver-specific defaults when resolving imports. Valid in node16, nodenext, and bundler.

Open questions

  • Should resolvePackageJsonExports and resolvePackageJsonImports be disableable in node16 and nodenext? I see no valid reason to disable them in those modes, but I haven’t yet prohibited it.
    • There was no objection to leaving these toggleable.
  • I would like to consider allowing *.ts imports to resolve in every module resolution mode, or at least node16 and nodenext, to improve consistency and (importantly) portability of .d.ts files between modes. I think @weswigham has already done this in another open PR, so perhaps it won’t be too controversial.
    • I think this is doable but not necessary for merging; will follow up in a subsequent PR.
  • With four new resolution-specific compiler options, it would be nice to introduce hierarchy into tsconfig.json. That’s a non-trivial design discussion on its own, especially considering that tsconfig files can inherit from each other. Is it worth tackling that first / as part of this / in the same release cycle to avoid the expansion of root-level options here?
    • This was not considered a blocker when I mentioned it in a design meeting. If there is time to design this before 5.0 is released, we can move the new options accordingly.
  • Should bundler become the new default resolution mode for --module commonjs? I would like to rename node to node10 in a follow-up PR, and stop maintaining it going forward. It is not a good choice for anyone since Node 10 is long out of service.
    • @sandersn endorsed this mode being the new default. I will get more feedback in a subsequent PR.

OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
X-link: facebook/metro#955

Pull Request resolved: facebook#36584

Changelog:
[General][Changed] - Default condition set for experimental Package Exports is now `['require', 'react-native']`

The [Exports RFC](https://github.com/react-native-community/discussions-and-proposals/blob/main/proposals/0534-metro-package-exports-support.md) had assumed that supporting the `"import"` condition was a syntax-only difference, given we are not in a Node.js environment — and so was worthwhile to support for maximal ecosystem compatibility.

{F915841105}

This assumption is similar to [`--moduleResolution bundler` in TypeScript 5.0](microsoft/TypeScript#51669):

> bundlers and runtimes that include a range of Node-like resolution features and ESM syntax, but do not enforce the strict resolution rules that accompany ES modules in Node or in the browser
> -- microsoft/TypeScript#51669 (comment)

However, robhogan has rightly pointed out that **we should not do this!**

- ESM (once transpiled) is **not** simply a stricter subset of in-scope features supported by CJS. For example, it supports top-level async, which would be breaking at runtime.
- We recently made the same change for our Jest environment:
    - facebook@681d7f8

As such, we are erring on the side of correctness and supporting only `['require', 'react-native']` in our defaults. At runtime, all code run by React Native is anticipated to be CommonJS. `"exports"` will instead allow React Native to correctly select the CommonJS versions of modules from all npm packages.

Metro changelog: [Experimental] Package Exports `unstable_conditionNames` now defaults to `['require']`

Reviewed By: robhogan

Differential Revision: D44303559

fbshipit-source-id: 0077e547e7775e53d1e4e9c3a9d01347f4fb7d4a
brawaru added a commit to brawaru/modrinth-knossos that referenced this pull request May 30, 2023
This is a new resolution algorithm in TypeScript that closely matches
the one present in popular bundlers like Vite and Webpack.

More details:
https://devblogs.microsoft.com/typescript/announcing-typescript-5-0-beta/#moduleresolution-bundler
microsoft/TypeScript#51669
@jedwards1211
Copy link

jedwards1211 commented Jun 22, 2023

@andrewbranch why does bundler apply the node export condition by default?

I've been trying to make a package with conditional exports that use node-fetch types for fetch, Response etc in node and Web API types otherwise, and it would be a lot easier to provide a good user experience if bundler, or some other mode, would not apply the node export condition. Because the only option is to tell users to set customConditions.

And node condition seems like a weird default for bundler since it's more common for bundlers to be used to deploy to web environments than to Node.

@andrewbranch
Copy link
Member Author

andrewbranch commented Jun 22, 2023

It doesn’t; #52940 removed it. I’ll update the PR description since docs still aren’t on the website (😬)

@jedwards1211
Copy link

Okay great! Thanks for letting me know

@andrewbranch
Copy link
Member Author

@jedwards1211 No problem. I assume you’re aware that Webpack, and I assume other bundlers too, do apply the node condition even though they’re not Node 🤦🏻‍♂️

@jedwards1211
Copy link

jedwards1211 commented Jun 23, 2023

@andrewbranch so I've learned recently...and even Deno does, somewhat surprisingly

@billti
Copy link
Member

billti commented Jul 8, 2023

I found this setting fixed a long running problem I had (yay!). In trying to learn more about it I checked out https://www.typescriptlang.org/tsconfig#moduleResolution , https://www.typescriptlang.org/docs/handbook/modules.html , and https://www.typescriptlang.org/docs/handbook/module-resolution.html and none of them cover it.

What's the best reference for details (this PR discussion is rather long)? Hopefully the docs will be updated soon. Thanks!

@andrewbranch
Copy link
Member Author

I’m updating the tsconfig.json reference now. It’s very hard to include bundler in the Module Resolution handbook page without totally rewriting it, which I’m essentially already doing over at #52593.

mklein994 added a commit to mklein994/playground-vue that referenced this pull request Jul 11, 2023
The release notes for TypeScript 5.0 recommend using "bundler" when
using tools like Vite, esbuild, Webpack, and so on.

- Docs (which unfortunately don't say much):
  https://www.typescriptlang.org/tsconfig#moduleResolution
- See also this PR: microsoft/TypeScript#51669
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
PR Backlog
  
Done