perf(napi): remove napi_build::setup() from oxc_napi to avoid redundant rebuilds#20094
perf(napi): remove napi_build::setup() from oxc_napi to avoid redundant rebuilds#20094
napi_build::setup() from oxc_napi to avoid redundant rebuilds#20094Conversation
github-actions
bot
added
the
C-performance
There was a problem hiding this comment.
Pull request overview
This PR removes the napi_build::setup() build script from the shared oxc_napi crate to prevent unnecessary rebuild invalidation during sequential napi build runs, relying on the leaf NAPI cdylib crates’ existing build.rs setup instead.
Changes:
- Deleted
crates/oxc_napi/build.rsto stop trackingNAPI_TYPE_DEF_TMP_FOLDERat the shared-library level. - Removed
napi-buildfromoxc_napibuild-dependencies and from the crate’s published include list. - Updated
Cargo.lockto drop thenapi-builddependency fromoxc_napi.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/oxc_napi/build.rs | Removed build script to avoid env-driven rebuild invalidation. |
| crates/oxc_napi/Cargo.toml | Dropped napi-build build-dependency and removed build.rs from include. |
| Cargo.lock | Removed napi-build from oxc_napi dependency list. |
Merging this PR will not alter performance
Comparing 
Footnotes
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1607b1c430
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…undant rebuilds `oxc_napi/build.rs` called `napi_build::setup()` which registers `cargo::rerun-if-env-changed=NAPI_TYPE_DEF_TMP_FOLDER`. Since each sequential `napi build` invocation sets a different value for this env var, `oxc_napi` was invalidated and recompiled for every NAPI package. `oxc_napi` is a library crate, not a cdylib entry point — the linker args and env tracking from `napi_build::setup()` are only needed in the leaf cdylib crates which already have their own `build.rs`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Boshen
force-pushed
the
perf/remove-napi-build-rs
branch
from
1607b1c to
bb2bd08
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb2bd082a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
### 🚀 Features - e8547cc parser: Report error for using declarations in ambient contexts (#19934) (camc314) - 8345318 allocator: Add methods for boxed slices `ArenaBox<[T]>` (#19968) (overlookmotel) - f83be30 allocator: Add `Vec::push_fast` method (#19959) (overlookmotel) ### 🐛 Bug Fixes - 291d867 transformer_plugins: Unwrap ChainExpression after define replacement removes optional markers (#20058) (IWANABETHATGUY) - 36b2e56 codegen: Print type for TSImportEqualsDeclaration (#20128) (camc314) - 5a246ec codegen: Print type arguments for JSXOpeningElement (#20127) (camc314) - a40870e codegen: Preserve parens for TSNonNullExpression (#20125) (camc314) - ae830b2 codegen: Print `declare` for `TSInterfaceDeclaration` (#20124) (camc314) - 92cfb14 linter/plugins: Fix types for `walkProgram` and `walkProgramWithCfg` (#20081) (overlookmotel) - ee0491e apps,napi: Explicitly specify libs in tsconfigs (#20071) (camc314) - 588009e codegen: Print `static` keyword for TSIndexSignature (#19755) (Dunqing) - 5a8799c codegen: Print `with_clause` for `ExportNamedDeclaration` (#20002) (Dunqing) - 7502afe parser: Correct capacity for tokens `Vec` (#19967) (overlookmotel) ### ⚡ Performance - 4ea8f9a napi: Remove `napi_build::setup()` from `oxc_napi` to avoid redundant rebuilds (#20094) (Boshen) - 2baa5fb napi: Unify build-test profile to coverage for cache sharing (#20090) (Boshen) - 8ba61dd parser: Make pushing tokens faster (#19960) (overlookmotel) Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
Summary
napi_build::setup()fromcrates/oxc_napi/build.rs(delete the file entirely) to preventoxc_napifrom being invalidated and recompiled on every sequentialnapi buildinvocationnapi_build::setup()registerscargo::rerun-if-env-changed=NAPI_TYPE_DEF_TMP_FOLDER, but@napi-rs/clisets a different value for this env var per package, causingoxc_napi+ all dependents to rebuild for each of the 5 NAPI packagesoxc_napiis a library crate — the linker args and env tracking are only needed in the leaf cdylib crates which already have their ownbuild.rs🤖 Generated with Claude Code