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

Upgrade TypeScript (and related) to 5.0, and start using package.json conditional exports #10

Merged
merged 9 commits into from
Apr 10, 2023

Conversation

milesrichardson
Copy link
Contributor

@milesrichardson milesrichardson commented Apr 10, 2023

Somewhat surprisingly, this all seems to be working.

Basically:

  • Upgrade TypeScript to latest
  • Opt out of verbatimModuleSyntax (but continue using isolatedModules: true)
  • Upgrade tsc-multi to latest get error with typescript 5 tommy351/tsc-multi#19
  • Continue targeting CJS outputs! :)
  • Use exports field of package.json, and specifically "conditional exports," so that published modules point to CJS or ESM (only using es2020 for ESM atm, although we build esnext) based on consumer, e.g.:
// print out the resolved path of a module imported via the module system
// (this flag is just necessary to get import.meta.resolve to print out the file)
// node --experimental-import-meta-resolve blah.mjs
console.log(await import.meta.resolve("@madatdata/react"));

// outputs:
// file:///path/to/madatdata/examples/react-nextjs-basic-hooks/node_modules/@madatdata/react/build/es2020/index.mjs

and

// node blah.js
console.log(require.resolve("@madatdata/react"));

// outputs:
// /path/to/madatdata/examples/react-nextjs-basic-hooks/node_modules/@madatdata/react/build/es2020-commonjs/index.cjs

Each package.json now includes this:

{
"exports": {
    ".": {
      "dev": "./index.ts", // <---- used by tsc customConditions and vitest resolve.conditions
      "require": {
        "types": "./build/es2020-commonjs/index.d.ts",
        "default": "./build/es2020-commonjs/index.cjs"
      },
      "import": {
        "types": "./build/es2020/index.d.ts",
        "default": "./build/es2020/index.mjs"
      },
      "default": {
        "types": "./build/es2020/index.d.ts",
        "default": "./build/es2020/index.mjs"
      }
    }
  }
}
  • Use a custom conditional export called dev which is not consumed by external tools (and therefore will not interfere with downstream packages), and use the new TypeScript 5.0 feature customConditions to make sure that TypeScript can resolve using moduleResolution: nodenext by seeing index.ts (instead of something in ./build for each package). Similarly, use a config flag resolutions.conditions in vitest, which passes it to vite, which passes it to node --conditions (see: Vitest doesn't handle package.json exports conditions like browser vitest-dev/vitest#2603 (comment))
  • Upgrade vitest to latest
  • Upgrade msw to latest and drop our hacky patch
  • Build latest versions of all packages and consume them with examples to make sure they still work

Once this is merged I'll publish the latest versions to npm.

Unfortunately I think skypack will still be broken, since it doesn't install dependencies of a package. But we'll see, maybe it will magically work too (doubt it).

…roken

Need to make some decisions about whether/how to emit CJS modules,
because as long as we have verbatimModuleSyntax enabled, we cannot
use the typical `export const = "x"` format, and tsc-multi rewriting
is also now breaking for this on v5, and patching it to do the rewrites
may be non trivial since it can involve shifting large blocks of code.

Alternatively we could just not opt into the flag, in which case
we should also restore the `isolatedModules` flag (which was removed
as redunant).

See:

* https://devblogs.microsoft.com/typescript/announcing-typescript-5-0/#verbatimmodulesyntax
* microsoft/TypeScript#52203
* tommy351/tsc-multi#19
However, vitest is not respecting the conditions flag, so right
now it's using `node` conditions flag in a way that would optimally
be `dev` with `customConditions` in `tsconfig.json`, but now any
downstream dependencies would try to use the `node` resolver too.
Will try upgrading vitest next.
…d patch)

Upgrade vitest to latest, and finally upgrade msw to the latest
version that supports Node 18 without our hacky patch.
Use "conditional exports" in `exports` field of package.json,
with an export named `dev` that is opt-in by `tsc` (via `tsconfig.base.json`)
and vitest (via `vitest.config.ts`, which passes the flag to `vite`, which passes
it to node using `--conditions` flag)

This is a new feature in TS5.0, and was fixed in vitest v0.23.0
(previously workers were not respecting the flag), so now we can
use it, which is good, because that leaves the `node` conditional
export to be filled by files from `dist` for the published config.
None of these will be used in dev (since we use `dev` for that),
but they should be consumed by downstream packages that install
madatdata packages from the registry, hopefully pointing them
to the right file.
…mples

Loop over each example and run `yarn install` (which I think is basically
a no-op, but whatever) and also delete any `.next` directory if it's
found in there. If any other future examples have similar cache directories
that should be cleaned up, then those directories should be added to this
script too, but hopefully the hack of a script will be gone by then.

Also update `examples/README.md` to reference the correct command
…opt out)

We're still opting out of `verbatimModuleSyntax`, but we can
continue using our `isolatedModules` config from prior to TS5.0.
@milesrichardson milesrichardson merged commit 0b45b8a into main Apr 10, 2023
@milesrichardson milesrichardson deleted the upgrades branch April 11, 2023 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant