Skip to content

[compiler] Migrate compiler packages to tsup #32550

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

Merged
merged 1 commit into from
Mar 7, 2025
Merged

[compiler] Migrate compiler packages to tsup #32550

merged 1 commit into from
Mar 7, 2025

Conversation

poteto
Copy link
Member

@poteto poteto commented Mar 7, 2025

Currently in the compiler workspace, we invoke esbuild directly to build most packages (with the exception of snap). This has been mostly fine, but does not allow us to do things like generate type declaration files.

I would like #32416 to be able to consume the merged eslint-plugin-react-compiler from source rather than via npm, and one of the things that has come up from my exploration in that stack using the compiler from source is that babel-plugin-react-compiler is missing type declarations. This is primarily because React's build process uses rollup + rollup-plugin-typescript, which runs tsc. So the merged plugin needs to typecheck properly in order to build. An alternative might be to migrate to something like babel with rollup instead to simply strip types rather than typecheck before building. The minor downside of that approach is that we would need to manually maintain a d.ts file for eslint-plugin-react-hooks. For now I would like to see if this PR helps us make progress rather than go for the slightly worse alternative.

tsup is esbuild based so build performance is comparable. It is slower when generating d.ts files, but it's still much faster than rollup which we used prior to esbuild. For now, I have turned off dts by default, and it is only passed when publishing on npm.

If you want to also generate d.ts files you can run yarn build --dts.

# BEFORE: build all compiler packages (esbuild)
$ time yarn build

✨  Done in 15.61s.
yarn build  13.82s user 1.54s system 96% cpu 15.842 total

# ---

# AFTER: build all compiler packages (tsup)
$ time yarn build

✨  Done in 12.39s.
yarn build  12.58s user 1.68s system 106% cpu 13.350 total

# ---

# AFTER: build all compiler packages and type declarations (tsup)
$ time yarn build --dts

✨  Done in 30.69s.
yarn build  43.57s user 3.20s system 150% cpu 31.061 total

I still need to test if this unblocks #32416 but this stack can be landed independently though as we could probably just release type declarations on npm. No one should be using the compiler directly, but if they really wanted to, lack of type declarations would not stop them (cf React secret internals).

Note that I still kept esbuild as we still use it directly for forgive.

Stack created with Sapling. Best reviewed with ReviewStack.

Currently in the `compiler` workspace, we invoke esbuild directly to build most packages (with the exception of `snap`). This has been mostly fine, but does not allow us to do things like generate type declaration files.

I would like #32416 to be able to consume the merged eslint-plugin-react-compiler from source rather than via npm, and one of the things that has come up from my exploration in that stack using the compiler from source is that babel-plugin-react-compiler is missing type declarations. This is primarily because React's build process uses rollup + rollup-plugin-typescript, which runs tsc. So the merged plugin needs to typecheck properly in order to build. An alternative might be to migrate to something like babel with rollup instead to simply strip types rather than typecheck before building. The minor downside of that approach is that we would need to manually maintain a d.ts file for eslint-plugin-react-hooks. For now I would like to see if this PR helps us make progress rather than go for the slightly worse alternative.

[`tsup`](https://github.com/egoist/tsup) is esbuild based so build performance is comparable. It is slower when generating d.ts files, but it's still much faster than rollup which we used prior to esbuild. For now, I have turned off `dts` by default, and it is only passed when publishing on npm.

If you want to also generate d.ts files you can run `yarn build --dts`.

```
# BEFORE: build all compiler packages (esbuild)
$ time yarn build

✨  Done in 15.61s.
yarn build  13.82s user 1.54s system 96% cpu 15.842 total

# ---

# AFTER: build all compiler packages (tsup)
$ time yarn build

✨  Done in 12.39s.
yarn build  12.58s user 1.68s system 106% cpu 13.350 total

# ---

# AFTER: build all compiler packages and type declarations (tsup)
$ time yarn build --dts

✨  Done in 30.69s.
yarn build  43.57s user 3.20s system 150% cpu 31.061 total
```

I still need to test if this unblocks #32416 but this stack can be landed independently though as we could probably just release type declarations on npm. No one should be using the compiler directly, but if they really wanted to, lack of type declarations would not stop them (cf React secret internals).

Note that I still kept esbuild as we still use it directly for forgive.
@poteto poteto merged commit 00aa004 into main Mar 7, 2025
20 of 21 checks passed
@poteto poteto deleted the pr32550 branch March 7, 2025 21:41
poteto added a commit that referenced this pull request Mar 7, 2025
Fixes an incorrect condition for running tests in the compiler
workspace.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32551).
* __->__ #32551
* #32550
BackEndTea added a commit to BackEndTea/react that referenced this pull request Mar 27, 2025
In facebook#32550 the modulex.exports was rewritten to `export` functionality.

However this removed the ability to do `import reactCompiler from "eslint-plugin-react-compiler"`
as described in the readme.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants