Conversation
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for nx-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
View your CI Pipeline Execution ↗ for commit e2aff44
☁️ Nx Cloud last updated this comment at |
leosvelperez
left a comment
There was a problem hiding this comment.
- We're missing the companion doc for the migration.
- Should this section in the docs be removed: https://nx.dev/docs/technologies/test-tools/vitest/guides/migrating-from-nx-vite#backward-compatibility?
I think we can keep the migration doc and say that |
There was a problem hiding this comment.
Nx Cloud has identified a flaky task in your failed CI:
🔂 Since the failure was identified as flaky, we triggered a CI rerun by adding an empty commit to this branch.
🎓 Learn more about Self-Healing CI on nx.dev
7d9c569 to
bdb960c
Compare
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
@nx/vite ships vitest version constants, the @nx/vite:test executor, the @nx/vite:vitest generator, and @nx/vite/plugin test target inference. Duplicates @nx/vitest. @nx/vite owns vite only. @nx/vitest owns vitest. Removed: - vitest version constants + helpers from versions.ts/version-utils.ts - @nx/vite:test executor - @nx/vite:vitest generator - @nx/vite/plugin test target inference + atomization - vitest install in @nx/vite init - vitest peer dep v23 safety-net migration installs @nx/vitest, swaps @nx/vite:test → @nx/vitest:test (project + targetDefaults), splits @nx/vite/plugin registrations, and registers @nx/vitest plugin alongside default-config @nx/vite/plugin (closes 22.2.0 gap). @nx/vite configuration generator with includeVitest still works — delegates to @nx/vitest via ensurePackage. Fixes NXC-4158
Co-authored-by: jaysoo <jaysoo@users.noreply.github.com>
@nx/vitest always installed, even in workspaces with no vitest usage. When @nx/vitest/plugin is already registered alongside @nx/vite/plugin entries that carry vitest options, those options are silently dropped. Migration guide backward-compat section still says "will be removed in Nx 23" rather than noting the removal happened. @nx/vitest installed only when vitest usage is detected: vitest in package.json, @nx/vite:test executor in any project, vitest-related options on @nx/vite/plugin, or vitest.config.* in any project root. Vitest options extracted from @nx/vite/plugin are merged into any existing @nx/vitest plugin entry (matched by include/exclude scope) without overwriting user-set fields. Migration guide backward-compat section updated to describe the removal as historical fact. Companion migration doc added under packages/vite/docs/. Addresses review comments on PR for NXC-4158
## Current Behavior @nx/vitest plugin registered alongside @nx/vite/plugin without gating package install on actual migration work. Workspaces that rely on @nx/vite/plugin test inference (no executor, no root vitest dep) silently lose test targets. ## Expected Behavior Each migration helper returns whether it changed anything. Install fires only when at least one helper actually did work. Adds an inference-only signal: glob vite.config.* / vitest.config.* and grep for top-level `test:` block. ## Related Issue(s) NXC-4158
## Current Behavior No migration path from @nx/vitest:test executor to the inferred @nx/vitest plugin. @nx/vite:convert-to-inferred no longer covers test (removed in the vitest split). ## Expected Behavior @nx/vitest:convert-to-inferred ports the test target migration: configFile → config, reportsDirectory → coverage.reportsDirectory, testFiles → testNamePattern. Mirrors the deleted @nx/vite version. ## Related Issue(s) NXC-4158
Migration uses a scopeKey helper to merge new @nx/vitest entries into existing ones with matching include/exclude. Defends against a scenario that doesn't realistically occur post-v22.2.0. Migration doc also lived under packages/vite/docs/ instead of next to the migration itself. Migration unconditionally splits vitest options off @nx/vite/plugin into a new @nx/vitest entry mirroring include/exclude. If a workspace ends up with duplicates, that mirrors a pre-existing config issue and is left visible. Migration doc moved to packages/vite/src/migrations/update-23-0-0/ to match repo convention. NXC-4158
## Current Behavior ensureVitestPluginRegistration short-circuits as soon as any @nx/vitest entry exists. Workspaces with mixed @nx/vite/plugin scopes (e.g. apps/**/* with testTargetName, libs/**/* bare) silently lose inference under the bare scope after migration: the apps split adds @nx/vitest, the global has-vitest check then bails before pairing libs. ## Expected Behavior Replace the global short-circuit with a per-scope coveredScopes set built from existing @nx/vitest entries. Each @nx/vite/plugin is paired with @nx/vitest unless its scope is already covered. ## Related Issue(s) NXC-4158
## Current Behavior Interface declared as the forEachExecutorOptions generic but the callback never reads _options. ## Expected Behavior Removed. Drops dead code without changing migration behavior. ## Related Issue(s) NXC-4158
migrateTargetDefaults had zero direct test coverage. The three patterns it handles (executor-keyed rename, target-name-keyed executor swap, collision merge) were untested. Three new specs covering each pattern. Collision case documents the legacy-wins Object.assign behavior intentionally. NXC-4158
## Current Behavior scopeKey takes PluginEntry, which is ExpandedPluginConfiguration<Record<string, unknown>>. Plugins read from nxJson come back as ExpandedPluginConfiguration<unknown>, which is wider on options. CI typecheck fails on the assignment. ## Expected Behavior scopeKey only reads include/exclude. Narrow the parameter to that structural shape so it accepts both PluginEntry and the wider read type without an unsafe cast. ## Related Issue(s) NXC-4158
…35576) ## Current Behavior Most Nx executors that have an inferred-plugin alternative (`@nx/<pkg>/plugin`) and a `convert-to-inferred` generator are still wired up like first-class citizens. There is no signal — at scaffold time, schema browsing, or task execution — that they are on a path to removal, and the existing `cypress`/`detox` deprecation messages are inconsistent with the canonical pattern shipped most recently. ## Expected Behavior Every executor that has an inferred-plugin migration target is now deprecated through three surfaces, matching the canonical pattern: - **Runtime warning.** The executor logs that it is deprecated, will be removed in Nx v24, and points at `nx g @nx/<pkg>:convert-to-inferred`. - **Schema-root `x-deprecated`.** Surfaces in editor / Nx Console / `nx show project` views. - **Generation-time warning.** When a generator is about to scaffold a target that uses one of these executors because the corresponding inferred plugin isn't registered, it warns at generation time and points at the same migration path. All warnings link to <https://nx.dev/docs/guides/tasks--caching/convert-to-inferred>. ### Executors deprecated in this PR | Package | Executors | |---|---| | `@nx/webpack` | `webpack`, `dev-server` | | `@nx/vite` | `build`, `dev-server`, `preview-server` | | `@nx/rollup` | `rollup` | | `@nx/next` | `build`, `server` | | `@nx/remix` | `build`, `serve` | | `@nx/jest` | `jest` | | `@nx/playwright` | `playwright` | | `@nx/eslint` | `lint` | | `@nx/storybook` | `storybook`, `build` | | `@nx/rspack` | `rspack`, `dev-server` | | `@nx/expo` | `build`, `export`, `install`, `prebuild`, `run`, `serve`, `start`, `submit` | | `@nx/react-native` | `build-android`, `build-ios`, `bundle`, `pod-install`, `run-android`, `run-ios`, `start`, `upgrade` | | `@nx/vitest` | `test` | ### Generation-time warnings wired in - `@nx/<pkg>:configuration` for webpack, vite, rollup, jest, playwright, storybook, rspack, vitest - `@nx/<pkg>:application` for next, expo, react-native - `@nx/eslint:lint-project` legacy fallback path - `@nx/react:application` (webpack, rspack branches) and `@nx/react:library` (rollup legacy fallback) — warning text is inlined rather than imported. Two distinct reasons: - **rspack:** `@nx/react` does not declare a tsconfig project reference to `@nx/rspack`, so the deep import would not even type-check. - **webpack and rollup:** the project reference exists, so the deep import compiles, but `@nx/webpack` and `@nx/rollup` only expose `./index.js` in their package `exports` field. The import would resolve in source but throw `Cannot find module '@nx/<pkg>/src/utils/deprecation'` at runtime in published packages. - `@nx/react-native:web-configuration` (webpack) — inline for the same reason as the rspack case (no project reference). ### Scope notes - `@nx/vite:test` is intentionally **not** included — it is being removed entirely by PR #35517 (deprecation messaging would ship as dead code). `@nx/vitest:test` is now in scope: the `convert-to-inferred` generator for `@nx/vitest` is in place, so the deprecation has a real migration target. - `@nx/cypress`/`@nx/detox` already shipped earlier; this PR retitles their generation-time messages to the new wording (drop the redundant "register the plugin first" guidance, swap "Scaffolding" for "Generating") and points the detox URL at the general convert-to-inferred guide. - `@nx/react-native:storybook` is intentionally **not** included. The companion `@nx/react-native:storybook-configuration` generator was already removed in v21 and no generator has emitted this target since Jan 2024 (RN 0.73 upgrade). It will be removed outright in a follow-up PR rather than going through the deprecate-now / remove-in-v24 cycle. - `@nx/webpack:ssr-dev-server`, `@nx/expo:build-list`, `@nx/expo:sync-deps`, `@nx/expo:update`, `@nx/expo:ensure-symlink`, `@nx/react-native:sync-deps`, and `@nx/react-native:ensure-symlink` are intentionally left as-is — none are covered by `convert-to-inferred` and they have no clean replacement (Nx-specific glue or non-migrating utilities). - `@nx/esbuild:esbuild` and `@nx/nuxt:*` ship neither an inferred plugin nor a `convert-to-inferred` generator yet, so they're out of scope. - Per-package READMEs, introduction-doc banners, per-package "migration recipe" pages, and `migrations.json` entries are intentionally skipped per the canonical pattern (NXC-4422). The runtime warning carries the migration story. ## Related Issue(s) Fixes NXC-4423. Fixes NXC-4296. Fixes NXC-4294. Fixes NXC-4290. Fixes NXC-4285. Fixes NXC-4283. Fixes NXC-4286. Fixes NXC-4297. Fixes NXC-4293. Fixes NXC-4292. Fixes NXC-4282. Fixes NXC-4287. Fixes NXC-4295. Fixes NXC-4447. --------- Co-authored-by: nx-cloud[bot] <71083854+nx-cloud[bot]@users.noreply.github.com>
|
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
Current Behavior
@nx/vitecarries a parallel vitest surface (version constants,:testexecutor,vitestgenerator, plugin test inference, init-time install) that duplicates@nx/vitest.Expected Behavior
@nx/viteowns vite.@nx/vitestowns vitest. Vitest surface removed from@nx/vite; consumers route through@nx/vitest.v23 migration installs
@nx/vitest, swaps the executor, and registers@nx/vitestplugin alongside default-config@nx/vite/pluginso test inference is preserved.Related Issue(s)
Closes NXC-4158