RTK ESM/TS Config meeting notes - 2023-02-27/28

rtk-esm-ts-notes-2023-02-27.md

RTK ESM/TS Discussion

Attendees:

• Nathan Bierema
• Mateusz Burzynski
• Mark Erikson

Notes

• Mateusz: what do you want besides "ship widely compat code?" What preferences?
  • Mark: don't care about .mjs as an output, really. Mostly just don't want to have to rewrite all imports to be "createSlice.js"
  • Mateusz: Could do some changes like rewriting imports as a post-build script
  • Mateusz: TS team kinda caved and will allow .ts extensions if you add a flag. Not sure what the output will be. Original reason for .js extensions was not wanting to transform source paths - "write what will be executed". So, in 5.0 they'll allow you to use .ts, question is whether they're going to rewrite that or not.
  • Mateusz: tried switching XState repo over, Jest had trouble with the .js extension due to mappings. Had to write a resolver plugin, and still had issues with .jsx/tsx too. So, not all tools might understand this depending on what the tool is looking for.
• Mateusz: "what does it mean to ship ESM today?"
  • Baseline: Node should be able to load files with require() and import, but you can even import CJS files (as long as it has named exports). Point is, you technically don't have to ship ESM for node, it can get loaded, and by shipping both formats you can run into the "dual-package hazard". (Possible to isolate shared code, but it has to be CJS. But, not every project suffers from that concern. Other issue is that files/deps can get double-imported, like two copies of Lodash.) So, shipping dual packages does make it more hazardous.
  • Mateusz: Probably do want to ship ESM files for bundlers, and that's where "export conditions" comes in. exports.module mostly understood by bundlers. ESM technically "async", but irelevant with bundlers because they're in control of the module graph and flattening things anyway. Some tools might even deduplicate require/module conditions? Not sure. Webpack 5 tries to be strict and follow Node semantics. So, by using exports.module, I as package author am being more clear about what I want tools to use.
  • Mateusz: So, technically, don't have to ship ESM just to get Node to work. So, to a certain extent, shipping ESM is about "purity". For now, Emotion is sticking with require, and bundlers will load ESM files.
• Mark: RTK ships pre-bundled versions in various formats (CJS/ESM/UMD). 1.9.x uses ESBuild+TSC (to compile to ES5), v2.0 sticks with esnext (although may transpile package.module to ES2017 for Webpack 4 parsing support). Redux core uses Rollup, other packages use Babel and output separate files.
  • Mateusz: TS doesn't even try to interpret require() as far as types. Lot of complexity around .cdts files, etc.
  • Mark: I'M LOST! :)
  • Mateusz: if you mark the whole package as type: "module", it gets a lot harder to have types for CJS consumers
  • Mark: I'm fine removing type: "module"
  • Mateusz: yeah, most important thing is the exports conditions.
  • Mark: so the order of fields in exports matters?
  • Mateusz: yes, matched from top to bottom. So, put types first (mentioned in the TS docs)
  • Mateusz: Andrew Branch (TS team) prefers to use "sibling files", but that's hard because TSC and other tools might be outputting in different ames/locations.
  • Mateusz: you're using typesVersions over in Reselect, right? TS will ignore typesVersions if you supply exports, but supports an equivalent version selection inside of exports.
  • Mark: we do stupid build shenanigans in Reselect with a renamed package.json to select between a nested type in TS 4.6 vs 4.7
  • Mateusz: this might not work with TS reading exports, TS might ignore it.
  • Mateusz: a lot of packages are misconfigured - they've started to use exports, but don't happen to change TS's moduleResolution field, so they don't see the issues
• Mark: shipped redux-thunk@3.0.0-alpha to switch from default export to only named; Reselect is only named. Would prefer to avoid shipping a React-Redux major just to alter packaging.
  • Mateusz: we did ship exports in an Emotion minor
  • Mark: everyone's warned me that shipping exports is a "breaking change"
  • Mateusz: we've decided that internal file structure is not a "public API". Yeah, some people can import from dist or src, but that's on them.
• Mark: been trying to set up a battery of RTK example apps (CRA, Next, Vite, Parcel, RN, etc), and build+run them in CI. Maybe they should live in a separate repo so that we can run the jobs in all the Redux repo CIs? (Would be really nice if this was generalized into a SAAS somehow...)
  • Mateusz: yeah, hard to do because of all the different flags and options, and also if external dependencies come into play.
• Mark: we've got 3 entry points, @reduxjs/toolkit, @reduxjs/toolkit/query, and @reduxjs/toolkit/query/react.
  • Mateusz: Yeah, Emotion has a couple as well. Nested package.json files is probably right. For exports, have multiple entries for "." and each sub-entry.
• Mark: What actions should we take?
  • Mateusz: I think it's easiest to only ship ESM for bundlers, which avoids the dual-package issue.
  • Mateusz: I provide ESM files for bundlers (see Emotion files):
    • exports.default -> CJS
    • exports.module -> ESM
    • Can combine exports conditions via nesting
  • Mark: What about TS "node16"?
    • Mateusz: Put "types" condition before "module"
  • Mark: any advantage to rolling up TS typedefs into one file?
    • Mateusz: not really. Save a couple FS reads, but no benefit there.
  • Mark: is it worth switching to a premade tool like preconstruct or tsup?
    • Mateusz: preconstruct isn't being marketed. Smart wrapper around Rollup, doesn't allow customization. Can do autofixes. Ran into trouble when Node shipped ESM support, hard to support dual packages out of the box.

tk-esm-ts-notes-2023-02-28.md

RTK ESM/TS Discussion with the TS Team

Attendees

• Mark Erikson
• Andrew Branch

Notes

• Andrew: any opening questions?
  • Mark: long rant about the state of package publishing and lack of public documentation
  • Andrew: yep. I worked on moduleResolution: "bundler", and I have a private repo instrumenting a half-dozen bundlers to understand what's going on
• Andrew: let's look at the RTK published output on unpkg:
  • Andrew: 2.0-alpha does pre-bundling of files per output format
  • Andrew: it's important that import specifiers match between TS and JS. Node can't handle ESM files without an extension. So, TS has to make assumptions about what the runtime will do.
  • Andrew: in this case, RTK's .d.ts files are actually a lie - the runtime part works fine because you've pre-bundled, but TS can't know that
  • Andrew: looking at transcript of discussion with Andarist, it may actually be a good idea to pre-bundle your .d.ts files too.
  • Andrew: the "don't change the module specifiers" issue is the deadest horse in the TS repo. But, at least in this case for RTK, it's ESBuild that matters for consuming the imports. This is actually what moduleResolution: "bundler" (coming in TS 5.0)
• Mark: trying to remember why I chose to pre-bundle all these different bundle output formats in the first place
  • Mark: Did try switching to individual files briefly with 2.0-alpha, but saw import errors with type: "module", gave up and went back to bundling
  • Andrew: users want to use TSC for varous output formats. Node CJS is relatively loose for imports, ESM is strict and wants extensions. But writing with extensions doesn't always work - default import/exports can cause problems.
  • Andrew: amazing how fast ESM syntax got adopted - added in ES2015, everyone used it immediately via Babel. Assumptions were made about behavior, and that's not what Node chose to do. Default imports can behave differently in Node CJS, ESM, and a bundler. TS needs to know about this because the type will be different. So, it might not be possible to write a file once, have TSC emit it, and have it work in both CJS/ESM. By bundling, you can get rid of a lot of those issues. So, bundling per entry point seems like a good idea.
  • Mark: supposedly shipping individual JS files is better for tree shaking, but anecdotally I don't see a problem with shipping bundles - our exports seem to get shaken okay
  • Andrew: if you want to ship multiple individual files, you could run TSC for each separate module mode. Can't just type-check once, emit two different outputs - you'd really need to type-check every time.
  • Andrew: can of course get into trouble with things like ESM-only dependencies
  • Mark: we have to assume our code runs everywhere - browser, Node, etc
• Andrew: been cringing every time I see advice about type: "module"
  • Andrew: There's nothing at the package level that says "this is an ESM package". type: "module" does only two things: tells Node how to interpret .js extensions, and also tells TS how to interpret .d.ts
  • Andrew: could ship type: "module", every file with .cjs - all files are CJS, that's what your extensions said, so type: "module" did nothing. Could flip it, omit type: "module" and every file is .mjs, again that field didn't matter. So, either you specify the format via file extension, or use type: "module" to say how .js files are interpreted. TS interprets .d.ts files the same way, because the compiler has to know what kind of module the types represent.
• Mark: actionable steps?
  • Mark: sounds like pre-bundling our own output is a good idea. Should also pre-bundle the TS types
  • Andrew: apparently api-extractor doesn't work with TS 5.0. Asked teammates, got conflicting answers on whether it's a dead project or not. May want to try a Rollup plugin? You may want to look at tsup in general - it does
• Mark: RTKQ relies on multiple entry points. 1.9.x uses nested package.json files, tried adding exports subpaths, seems to mostly work.
  • Mark: would be fantastic if we had more automated tools for checking ESM compat, like "here's my app code, generate and build a dozen projects with different tooling", or enhanced versions of publint and arethetypeswrong

We wrote a custom transformer on top of the TS compiler API. Basically it uses outFile to emit a single file with the types for all dependencies in module blocks, and then runs a transform to flatten it and shake out types that aren't reachable from the entry point. It's not perfect, and there have been some bugs with some really complex types, but it works pretty well. Ideally TS itself would have something like this we could just call though.

We did work on it once, but we ended up rejecting the feature - it's too tied to the concept of bundling to be useful without significant additional complexity in the compiler itself (it's basically building a full bundler into the compiler and its' options, and we really don't have the bandwidth to write and maintain a full bundler, even if we wanted to); you need to know how all of the including, splitting, and inlining is being done in the js bundle (for each entrypoint) to match that behavior in the .d.ts bundle.

.d.ts bundlers tied to .js bundlers is, imo, the best direction to go, since that's where the tools all know how everything should fit together (at least without significant duplicated configuration and logic that introduces another point of configuration failure).

Completely off-topic aside for anyone working on a .d.ts bundler feature of a bundler, though: please also transform the declaration maps like you do the js source maps in a bundler if you can, thanks. I like it when they can go-to-def all the way back to the original source-code-as-written :)

There should be no question about this. allowImportingTsExtensions requires you to set noEmit or emitDeclarationOnly. All the relevant PRs, issues, and blog posts have been very clear that tsc is not going to rewrite a .ts extension in a module specifier to a .js one.

Yeah, that was my overall guess - but I wasn't 100% sure about it. It makes sense and fits the overall story behind TS's decisions here and the fact that it is not a bundler. I mentioned during the call that this doesn't even matter that much for Mark since he is often not using tsc to emit .js files, so it's Rollup that has to "rewrite" those (and it does it with no extra configuration).

What I assumed Mateusz meant was specifically the const foo = require("...") case.

Yes, that's what I meant.

Also, in module emit modes where import foo from "..." is emitted as a require call, we understand that it’s going to be a require and check and resolve appropriately, including setting the require condition for package.json exports resolving. Not sure how or if any of this is relevant to your use case without more context.

Right, I tried to explain this exactly during the call - mentioning that the fact that the code is authored in ESM doesn't exactly imply that TS interprets it as ESM. That's the exact reason why type: 'module'+ exports.types is a dangerous~ combination if the package is meant to be consumed from CJS too.

We wrote a custom transformer on top of the TS compiler API. Basically it uses outFile to emit a single file with the types for all dependencies in module blocks, and then runs a transform to flatten it and shake out types that aren't reachable from the entry point. It's not perfect, and there have been some bugs with some really complex types, but it works pretty well. Ideally TS itself would have something like this we could just call though.

The main use case for "tree-shaking" typedefs is the library code. But at that point... such tree-shaken types are completely redundant. Wouldn't it be better to have a linter~ that would detect such unused types instead of "tree-shaking" them?