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

build: update packages to use more modern moduleresolution options #1770

Conversation

michaelfaith
Copy link
Contributor

@michaelfaith michaelfaith commented Dec 19, 2024

Following: #1760 (comment)

This change updates tsconfigs across all packages to use more up to date module and moduleResolution options. For all packages that were being built as commonjs modules with node moduleResolution, they've been updated to use nodenext.

I also took the opportunity to re-organize the tsconfigs at the packages level to make their names more accurate. Previously the main packages/tsconfig.json, which is what the build:esm command targeted, referenced all package's tsconfigs, while a packages/tsconfig.cjs.json only referenced the cjs tsconfig from the react package. However, all packages' tsconfigs except for the react tsconfig are building cjs. So having a command that's called build:esm run all cjs builds and one esm build didn't really do what the name would suggest. So I reversed that construct. Now packages/tsconfig.json still references all of the packages tsconfig, except for the react package, it references the tsconfig.cjs, and I renamed the packages/tsconfig.cjs.json to packages/tsconfig.esm.json and have it only referencing the main react package tsconfig (which is the only esm config).

The net / net is that build:esm is only building esm output and build:cjs is only building cjs output.

Similarly, all package level tsconfigs are inheriting from packages/tsconfig.options.json, but that config had module and moduleResolution corresponding to the tsconfig in react (the ESM one), and all of the other package tsconfigs were overriding the module and moduleResolution. So I swapped that, so that the base config has the module and moduleResolution that all but on of the packages are using, and had the react tsconfig override that to what it needed to be for esm output. This means fewer tsconfigs are needing to override those options.

Lastly, I've done build comparisons for all packages and targets, and all result in the exact same build output as they did before these changes, except for one file. utils/with-plugin which has a dynamic import. The old school commonjs module didn't support dynamic imports so it transpiled it down to a require and a bunch of additional wiring to give a similar behavior. In nodenext it just uses the dynamic import directly.

(left is before, right is after)

All this extra cruft:
image

Just to support this one import
image

So, I'm not sure how you'd like to represent this in the utils changelog. All the rest are exactly the same output.

image

This change updates tsconfigs across all packages to use more up to date `module` and `moduleResolution` options.  For all packages that were being built as `commonjs` modules with node `moduleResolution`, they've been updated to use `nodenext`.

I also took the opportunity to re-organize the tsconfigs at the packages level to make their names more accurate.  Previously the main `packages/tsconfig.json`, which is what the `build:esm` command targetted, referenced all package's tsconfigs, while a `packages/tsconfig.cjs.json` only referenced the cjs tsconfig from the react package.  However, all packages' tsconfigs _except_ for the react tsconfig are building cjs.  So having a command that's called `build:esm` run all cjs builds and one esm build didn't really do what the name would suggest.  So I reversed that construct.  Now `packages/tsconfig.json` still references all of the packages tsconfig, except for the react package, it references the `tsconfig.cjs`, and I renamed the `packages/tsconfig.cjs.json` to `packages/tsconfig.esm.json` and have it only referencing the main react package tsconfig.

The net / net is that `build:esm` is only building esm output and `build:cjs` is only building cjs output.

Similarly, all package level tsconfigs are inheriting from packages/tsconfig.options.json, but that config had module and moduleResolution corresponding to the tsconfig in react (the ESM one), and all of the other package tsconfigs were overriding the module and moduleResolution.  So I swapped that, so that the base config has the module and moduleResolution that all but on of the packages are using, and had the react tsconfig override that to what it needed to be for esm output. This means fewer tsconfigs are needing to override those options.

Lastly, I've done build comparisons for all packages and target, and they all result in the same build output as they did before these changes.
Copy link

changeset-bot bot commented Dec 19, 2024

⚠️ No Changeset found

Latest commit: 8e8e159

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

netlify bot commented Dec 19, 2024

Deploy Preview for compiled-css-in-js canceled.

Name Link
🔨 Latest commit 8e8e159
🔍 Latest deploy log https://app.netlify.com/sites/compiled-css-in-js/deploys/677df90accfcb2000834e968

@dddlr dddlr enabled auto-merge (squash) January 8, 2025 04:03
@dddlr dddlr merged commit 5da86c5 into atlassian-labs:master Jan 8, 2025
14 checks passed
@michaelfaith michaelfaith deleted the build/update-tsconfig-module-resolution branch January 8, 2025 09:46
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.

3 participants