Migrate napi-rs bindings from v2 to v3 - #95412
Merged
Merged
Conversation
Contributor
Tests PassedCommit: 90d08aa |
Contributor
Stats from current PR✅ No significant changes detected📊 All Metrics📖 Metrics GlossaryDev Server Metrics:
Build Metrics:
Change Thresholds:
⚡ Dev Server
📦 Dev Server (Webpack) (Legacy)📦 Dev Server (Webpack)
⚡ Production Builds
📦 Production Builds (Webpack) (Legacy)📦 Production Builds (Webpack)
📦 Bundle SizesBundle Sizes⚡ TurbopackClient Main Bundles
Server Middleware
Build DetailsBuild Manifests
Build Cache
📦 WebpackClient Main Bundles
Polyfills
Pages
Server Edge SSR
Middleware
Build DetailsBuild Manifests
Build Cache
🔄 Shared (bundler-independent)Runtimes
📝 Changed Files (2 files)Files with changes:
View diffspages-api.ru..time.prod.jsDiff too large to display pages.runtime.prod.jsDiff too large to display 📎 Tarball URLCommit: 90d08aa |
jimmyhmiller
force-pushed
the
jimmym/napi-v3-migration
branch
2 times, most recently
from
July 10, 2026 15:49
5b37996 to
9453258
Compare
jimmyhmiller
marked this pull request as ready for review
July 10, 2026 16:45
bgw
reviewed
Jul 11, 2026
Comment on lines
+105
to
+109
| # Newest @napi-rs/cli that runs on Node.js 20.9.0 (NODE_LTS_VERSION below): | ||
| # 3.5.0+ depends on @inquirer/prompts 8, whose @inquirer/core 11 requires | ||
| # `util.styleText` (Node >= 20.12) and crashes at CLI startup. The CLI here | ||
| # only orchestrates cargo and names artifacts (types are committed), so | ||
| # drifting from the workspace devDependency (3.7.2) is acceptable. |
Member
There was a problem hiding this comment.
Asking here: https://vercel.slack.com/archives/C04KC8A53T7/p1783709048113789
If we have to do this, let's make everything use version 3.4.1 of the CLI? I don't like the idea of mixing these versions between jobs if there's not a really good reason we can't just pin on 3.4.1?
bgw
requested changes
Jul 13, 2026
This was referenced Jul 14, 2026
jimmyhmiller
added a commit
that referenced
this pull request
Jul 27, 2026
…Ref types (#95875) ## Summary Follow-up to two review comments on #95412: - **Type the subscribe callbacks.** The seven `*Subscribe` / `*Changed` bindings declared their callback as `(...args: any[]) => any`. They now use error-first typed signatures, e.g. `(err: Error, value: TurbopackResult<UpdateMessage>) => void`. (bgw: _"clean up these typescript `any` types wherever we reasonably can"_.) - **Tighten the Rust `FunctionRef` value types.** `project_update_info_subscribe` and `project_compilation_events_subscribe` still used `FunctionRef<Unknown<'static>>`; they now use their concrete mapper output (`NapiUpdateMessage` / `NapiCompilationEvent`). `project_hmr_events` intentionally keeps `Unknown` since it emits a dynamic `to_js_value` client/server union. (bgw: _"get rid of `Unknown` here ... that'll involve updating callsites too"_.) `generated-native.d.ts` was regenerated from these Rust changes via `pnpm swc-build-native`. That regeneration also refreshes a handful of lines that were stale in the committed copy (a few `Promise<undefined>` → `Promise<void>` and one doc comment) — these reflect what the repo's pinned CLI emits today, not hand edits. ## Verification - `pnpm --filter=next types` <!-- NEXT_JS_LLM_PR -->
bgw
approved these changes
Jul 29, 2026
bgw
approved these changes
Aug 3, 2026
Member
|
Maybe hold off on merging this until we figure out if 16.3.x is going to get cut from |
jimmyhmiller
added a commit
that referenced
this pull request
Aug 12, 2026
…Ref types Addresses two follow-up review comments from #95412: - Replace the `(...args: any[]) => any` `ts_arg_type` on the seven subscribe/changed callbacks with error-first typed signatures (`(err: Error, value: TurbopackResult<...>) => void`). - Tighten the Rust `FunctionRef` value types that were still `Unknown` to the concrete mapper output (`NapiUpdateMessage`, `NapiCompilationEvent`). `project_hmr_events` keeps `Unknown` because it emits a dynamic `to_js_value` union. Regenerating generated-native.d.ts with the repo's pinned CLI also refreshes a few lines that were stale in the committed copy.
jimmyhmiller
force-pushed
the
jimmym/napi-v3-migration
branch
from
August 12, 2026 13:52
f814bb9 to
3b307ed
Compare
Bump the workspace to napi 3.10.0 / napi-derive 3.5.8 / @napi-rs/cli 3.7.2 and port all first-party bindings to the modern (compat-mode-free) API: ThreadsafeFunction builder, Object/Function/Unknown value types, FunctionRef::borrow_back, External/ExternalRef, and #[napi(object)] outputs. Drop the napi-2-pinned lightningcss-napi crate and vendor it in-repo as turbopack-lightningcss-napi, rewritten to the modern napi 3 API. This also fixes the visitor bridge, which silently no-op'd on v3 because the compat JsFunction::try_from(Unknown) path failed (breaking CSS url()/@import rewriting and CSS-module handling). Update the @napi-rs/cli v3 build config (binaryName/targets, --manifest-path/--no-js/-o) and align @emnapi/* to 1.11.1 for the wasi build.
The v2 migration changed the build-native* scripts to v3 CLI flags (--manifest-path/--no-js/-o), but CI still installed @napi-rs/cli 2.18.4, which rejects --manifest-path. Bump the pinned CLI to 3.7.2 in the native builder Dockerfile and the NAPI_CLI_VERSION env across the build/release workflows so it matches the workspace devDependency.
napi v3's External<T> is !Send, so the async #[napi] fns that took an
External needed an unsafe Send/Sync wrapper (SendableExternalRef) to satisfy
spawn_future's Send-future bound. Instead, make those functions synchronous:
take &External, extract the Send data (turbopack ctx / container / endpoint
op) up front, and return env.spawn_future(async move { ... }) as PromiseRaw.
The !Send reference stays confined to the sync body and never crosses into
the future, so the unsafe wrapper is deleted. JS behavior is unchanged
(still returns a Promise).
Correctness/robustness: - Drop return_if_invalid from project_new so invalid JS arguments throw a TypeError again instead of silently resolving to undefined. - Take the project exit receiver inside the spawned future (after stop_and_wait in project_shutdown), restoring the v2 ordering: a panic in stop_and_wait no longer consumes the receiver, so exit handlers survive for the retry/fallback onExit call. Share the take+run logic between project_on_exit and project_shutdown via an Arc<std::sync::Mutex<..>>. - Restore lockfile_unlock's async offload: release the lock on the blocking pool via env.spawn_future + spawn_blocking instead of the JS thread. Latent hazards: - Replace the Object::from_raw 'static lifetime detach in project_compilation_events_subscribe with a NapiCompilationEvent object struct, matching the NapiUpdateMessage pattern. - Tie the vendored lightningcss crate's 'env lifetimes to the &Env argument so returned Unknowns are actually scope-bound. - Replace deprecated ThreadsafeFunction::unref with the Weak const generic (same napi_unref_threadsafe_function call, no deprecation warnings). Cleanup: - Re-add the opt-level="s" release override for the vendored turbopack-lightningcss-napi crate (lost with the lightningcss-napi dep). - Restore &'static str fields on output-only object structs (NapiRoute, NapiUpdateMessage, NapiSourceDiagnostic), dropping per-message allocs. - Add a provenance README to the vendored crate (upstream version, local modifications, drop criteria). Types: - Regenerate generated-native.d.ts with the v3 CLI. Add ts_arg_type/ ts_return_type overrides where v3 typegen is weaker than the runtime contract (subscribe callbacks, worker scheduler tsfns, Option returns, entrypoints results), drop the manual header's stale lightningCss declarations (typegen emits them now), and correct the css.lightning facade types to reflect that the native transforms are synchronous.
@napi-rs/cli 3.x (via @inquirer/core) imports util.styleText, which was added in Node.js 20.12.0. The build_reusable jobs ran Node 20.9.0, so every build-native job crashed at CLI startup with 'does not provide an export named styleText'. All other workflows already float on Node 20/22.
Reverts the NODE_LTS_VERSION bump. @napi-rs/cli 3.5.0+ depends on @inquirer/prompts 8, whose @inquirer/core 11 imports util.styleText (Node >= 20.12) and crashes at CLI startup on the pinned Node 20.9.0. 3.4.1 is the newest CLI on the @inquirer/prompts 7 line (engines >=18); verified locally under Node 20.9.0: 3.7.2 reproduces the exact CI crash, 3.4.1 builds the addon and generates types successfully. Only build_reusable.yml runs the CLI under NODE_LTS_VERSION; the other workflows and the native-builder image resolve Node 20 latest and stay on 3.7.2, matching the workspace devDependency used for typegen.
… JS thread napi v2 permanently entered its tokio runtime context on the addon's main thread (a leaked EnterGuard at module init). Both next-napi-bindings and turbo-tasks (e.g. PriorityRunner::spawn) call tokio::spawn from synchronous N-API entrypoints and relied on that ambient context. napi v3 removed the permanent enter, so any 'next dev' aborted with 'there is no reactor running' as soon as a subscription or turbo-tasks work was scheduled from the JS thread (first surfaced in projectCompilationEventsSubscribe, then in turbo-tasks' priority_runner). Restore the v2 behavior in module_init: capture the custom runtime's handle via within_runtime_if_available (which also forces napi to adopt the custom runtime) and leak an EnterGuard for the lifetime of the thread. Also wrap the two bare tokio::spawn calls in the subscribe entrypoints in within_runtime_if_available for defense in depth. Verified locally: 'next dev' on an app-dir fixture serves HTTP 200 with no panic; previously it aborted at startup.
Use the re-exported napi::Unknown instead of spelling out the full napi::bindgen_prelude::Unknown path.
There are many napi types with both Js-prefixed and unprefixed variants, so aliasing Unknown to JsUnknown was confusing. Use the plain name.
Drop the JsUnknown alias and thread the concrete napi value type V through subscribe() and its FunctionRef, instead of erasing the callback argument to Unknown.
External<T> is Send, so these don't need env.spawn_future or an unsafe SendableExternalRef wrapper. endpoint_write_to_disk goes back to a plain async fn taking &External, the two subscribe entrypoints take &External instead of ExternalRef, FunctionRef is imported, JsUnknown is dropped, and the subscribe callbacks get concrete value types.
Same as the endpoint change: replace the env.spawn_future wrappers with plain async fns taking &External<ProjectInstance>. Also use the already imported PromiseRaw, drop the JsUnknown alias and the migration meta-comments, and give the subscribe callbacks concrete value types.
- Drop leftover clones in endpoint_write_to_disk, project_update, project_on_exit/project_shutdown, and the exit_receiver field (no longer needs Arc now nothing spawns a 'static future off it). - Revert trivially-inferred Ok::<_, napi::Error>(()) annotations. - Inline project_write_all_entrypoints_to_disk_inner, which had only one caller. - Move TransformOutputResult (and its only builder, complete_output) into transform.rs where they're used; re-export from lib.rs. - Fix rspack.rs FinderTask::resolve to name its unused env param instead of a dead `let _ = env;` statement.
- Generalize the take_lockfile_inner poison message; it's reached from both lockfile_unlock_sync and lockfile_unlock now. - Import Mutex directly in project.rs instead of writing std::sync::Mutex three times (no tokio::sync::Mutex left to disambiguate against). - Drop redundant type annotations/turbofish in turbopack_ctx.rs and utils.rs that are already fully inferred. - Match the rest of turbopack_ctx.rs by using the imported env::var instead of std::env::var. - Trim a doc comment referencing "the previous manual object construction," which no longer exists in the codebase.
- Drop the two stale #[allow(dead_code)] on the pub-exported
lightningCssTransform/lightningCssTransformStyleAttribute wrappers;
keep it on the private mask fn, which really does need it (verified
with clippy --all-targets: it's only reachable via napi macro
registration, invisible to the test-build target).
- e.to_string() instead of format!("{}", e) in the same file.
- Match Ok(()) instead of Ok(_) in throw_turbopack_internal_error,
since call_async's Return generic here is unit.
- Sink env_wrapper into the branch that actually uses it in the
TurbopackResult ToNapiValue impl.
- Fix a double space in a comment.
…Ref types Addresses two follow-up review comments from #95412: - Replace the `(...args: any[]) => any` `ts_arg_type` on the seven subscribe/changed callbacks with error-first typed signatures (`(err: Error, value: TurbopackResult<...>) => void`). - Tighten the Rust `FunctionRef` value types that were still `Unknown` to the concrete mapper output (`NapiUpdateMessage`, `NapiCompilationEvent`). `project_hmr_events` keeps `Unknown` because it emits a dynamic `to_js_value` union. Regenerating generated-native.d.ts with the repo's pinned CLI also refreshes a few lines that were stale in the committed copy.
…e-interfaces lint
- NapiCompilationEvent::type_name is &'static str instead of an allocated String; CompilationEvent::type_name already returns &'static str and napi handles it fine. - Use expect_err instead of a match for the internal-error callback, which must always throw. - Annotate the ThreadsafeFunction const generics in turbopack_ctx.rs with /* CalleeHandled */ and /* Weak */ comments, and let inference handle the two local bindings whose builder chains already pin those values. - Drop the napi v2 -> v3 narration from worker_thread.rs. - lightningcss transform/transformStyleAttr have always been synchronous; drop the stale await in the minify plugin and the comment excusing it. - Note that leaking the tokio runtime guard keeps the runtime alive for the process lifetime, to be fixed in a follow-up. - Remove the @emnapi/* pnpm overrides. Nothing in our napi build reads them; they only repinned @rspack/binding-wasm32-wasi's emnapi deps.
This plugin has never been instantiated. It arrived in #61327, which forked fz6m/lightningcss-loader; that package ships both a loader and a minifier plugin, and Next only ever wired up the loader (webpack/config/blocks/css/loaders/{global,modules}.ts). CSS minification goes through CssMinimizerPlugin for webpack and rspack's own LightningCssMinimizerRspackPlugin, neither of which touches this file. No commit in the repo's history constructs it, and the only references were its own definition plus the re-export in index.ts. Also drops ECacheKey.minify, which only the plugin used. Note this removes the class from the compiled next/dist/build/webpack/loaders/lightningcss-loader/src/index.js. That is a build-internals deep import, never exported from the package root or documented, so it is not a supported surface.
jimmyhmiller
force-pushed
the
jimmym/napi-v3-migration
branch
from
August 12, 2026 17:11
d1458bd to
90d08aa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft PR for testing