Skip to content

Commit ab9a0b3

Browse files
committed
fix(migrate): 4 PR-review fixes (bridge resolve, force-stage, prettier gate, preview registry)
Addresses four Codex review comments on PR #1891, all verified valid: - install.sh: `resolve_bridge_commit_version ... || true` so `set -e` does not abort at the assignment before the "Could not resolve" error prints (the installer otherwise fails silently on an unregistered/transient bridge miss). - test-pkg-pr-new-migrate.sh: only force-stage `.npmrc`/`.yarnrc.yml` that carry the bridge marker, not a pre-existing ignored file that may hold private registry auth tokens the migration never touched. - migration/bin.ts: detect Prettier unconditionally on existing-Vite+ upgrades so the format gate skips Oxfmt on a Prettier project even on a non-`--full` upgrade; the Prettier migration itself stays gated on `--full`. - preview-registry.ts: thread the detected package manager into reconcilePreviewBridgeRegistry so a pnpm/npm/bun project with a stray `.yarnrc.yml` still gets `.npmrc` (its install reads `.npmrc`). Adds a test. Claude-Session: https://claude.ai/code/session_01DQhS6o1fyQd1yjiee6W8jR
1 parent 63f877e commit ab9a0b3

6 files changed

Lines changed: 44 additions & 15 deletions

File tree

.github/scripts/test-pkg-pr-new-migrate.sh

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,8 +299,14 @@ if [ "$is_git_repo" -eq 1 ]; then
299299
# the project's CI: the commit build then resolves from the default registry,
300300
# which has no 0.0.0-commit.<sha>, and the supply-chain policy check rejects the
301301
# lockfile (ERR_PNPM_TARBALL_URL_MISMATCH). Stage whichever file migrate wrote.
302-
git -C "$project_dir" add -f .npmrc 2>/dev/null || true
303-
git -C "$project_dir" add -f .yarnrc.yml 2>/dev/null || true
302+
# Only stage config that carries the bridge marker, i.e. was written for this
303+
# run. A pre-existing ignored `.npmrc`/`.yarnrc.yml` (private-registry auth
304+
# tokens) that migrate never touched must not land in the migration diff.
305+
for cfg in .npmrc .yarnrc.yml; do
306+
if [ -f "$project_dir/$cfg" ] && grep -q "registry-bridge.viteplus.dev" "$project_dir/$cfg" 2>/dev/null; then
307+
git -C "$project_dir" add -f "$cfg" 2>/dev/null || true
308+
fi
309+
done
304310
echo
305311
echo "Migration worktree changes (.npmrc force-staged so it survives .gitignore):"
306312
git -C "$project_dir" status --short

packages/cli/install.sh

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -898,7 +898,10 @@ main() {
898898
# immutable commit version (0.0.0-commit.<sha>), the clearly-defined test
899899
# version we install. The directory label stays non-semver so it keeps out
900900
# of cleanup_old_versions and makes the PR build obvious in `~/.vite-plus/`.
901-
PR_COMMIT_VERSION="$(resolve_bridge_commit_version "$PR_VERSION")"
901+
# `|| true` keeps `set -e` from aborting this assignment when resolution
902+
# fails (unregistered ref / transient bridge error), so the actionable
903+
# error below is reachable instead of the installer exiting silently.
904+
PR_COMMIT_VERSION="$(resolve_bridge_commit_version "$PR_VERSION" || true)"
902905
if [ -z "$PR_COMMIT_VERSION" ]; then
903906
error "Could not resolve a registry bridge build for ${PR_VERSION}"
904907
fi

packages/cli/src/migration/bin.ts

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1291,18 +1291,16 @@ async function main() {
12911291
eslintMigrated = true;
12921292
}
12931293

1294-
// Prettier migration is part of the full setup bucket. Default upgrades skip
1295-
// detection entirely; the variables stay at their no-prettier defaults so the
1296-
// formatting gate and summary below still compile and behave correctly.
1297-
let prettierProject: { hasDependency: boolean; configFile?: string } = {
1298-
hasDependency: false,
1299-
};
1294+
// Detect Prettier unconditionally so the formatting gate below skips Oxfmt on
1295+
// a project that still uses Prettier, even on a bare (non-`--full`) upgrade
1296+
// that rewrites imports/scripts. The Prettier MIGRATION itself stays in the
1297+
// full-setup bucket.
1298+
const prettierProject = detectPrettierProject(
1299+
workspaceInfoOptional.rootDir,
1300+
workspaceInfoOptional.packages,
1301+
);
13001302
let prettierMigrated = false;
13011303
if (fullSetup) {
1302-
prettierProject = detectPrettierProject(
1303-
workspaceInfoOptional.rootDir,
1304-
workspaceInfoOptional.packages,
1305-
);
13061304
if (prettierProject.hasDependency && prettierProject.configFile) {
13071305
// Interactive only: stop any active spinner (e.g. "Migrating ESLint") so
13081306
// it does not animate beneath the confirm prompt.

packages/cli/src/utils/__tests__/preview-registry.spec.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import path from 'node:path';
44

55
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
66

7+
import { PackageManager } from '../../types/index.ts';
78
import { isPreviewVitePlusVersion, reconcilePreviewBridgeRegistry } from '../preview-registry.ts';
89

910
const PREVIEW = '0.0.0-commit.81dee3abe99a61a28ebf112da4f76f2a32aec8b4';
@@ -49,6 +50,16 @@ describe('reconcilePreviewBridgeRegistry', () => {
4950
expect(fs.existsSync(path.join(dir, '.yarnrc.yml'))).toBe(false);
5051
});
5152

53+
// PR #1891 review: a non-Yarn package manager must get `.npmrc` even when a
54+
// stray `.yarnrc.yml` is present, or its install reads `.npmrc` and can't
55+
// resolve `0.0.0-commit.<sha>` from the default registry.
56+
it('writes .npmrc for a non-Yarn package manager despite a stray .yarnrc.yml', () => {
57+
fs.writeFileSync(path.join(dir, '.yarnrc.yml'), 'nodeLinker: node-modules\n');
58+
expect(reconcilePreviewBridgeRegistry(dir, PREVIEW, PackageManager.pnpm)).toBe(true);
59+
expect(fs.existsSync(path.join(dir, '.npmrc'))).toBe(true);
60+
expect(fs.readFileSync(path.join(dir, '.npmrc'), 'utf8')).toContain(`registry=${BRIDGE}`);
61+
});
62+
5263
it('appends to an existing .npmrc without clobbering it', () => {
5364
fs.writeFileSync(path.join(dir, '.npmrc'), '@scope:registry=https://npm.example.com/\n');
5465
reconcilePreviewBridgeRegistry(dir, PREVIEW);

packages/cli/src/utils/preview-registry.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import fs from 'node:fs';
22
import path from 'node:path';
33

4+
import { PackageManager } from '../types/index.ts';
45
import { VITE_PLUS_VERSION } from './constants.ts';
56
import { readJsonFile } from './json.ts';
67
import { editYamlFile } from './yaml.ts';
@@ -159,9 +160,19 @@ function clearYarnBerryRegistry(projectRoot: string): boolean {
159160
export function reconcilePreviewBridgeRegistry(
160161
projectRoot: string,
161162
version: string = process.env.VP_VERSION || VITE_PLUS_VERSION,
163+
packageManager?: PackageManager,
162164
): boolean {
163165
if (isPreviewVitePlusVersion(version)) {
164-
if (isYarnBerryProject(projectRoot)) {
166+
// Write the file the ACTIVE package manager reads: Yarn Berry uses
167+
// `.yarnrc.yml`, everything else uses `.npmrc`. Fall back to file-based
168+
// detection only when the manager is unknown, so a stray leftover
169+
// `.yarnrc.yml` in a pnpm/npm/bun project doesn't leave `.npmrc` without the
170+
// bridge registry (the install would then fail to resolve `0.0.0-commit.<sha>`).
171+
const useYarnBerry =
172+
packageManager === PackageManager.yarn
173+
? isYarnBerryProject(projectRoot)
174+
: packageManager === undefined && isYarnBerryProject(projectRoot);
175+
if (useYarnBerry) {
165176
ensureYarnBerryRegistry(projectRoot);
166177
} else {
167178
ensureNpmrcRegistry(projectRoot);

packages/cli/src/utils/prompts.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ export async function runViteInstall(
116116
// run left behind so installs don't hit the test bridge. No-op for a real build
117117
// with no leftover. Done before the VP_SKIP_INSTALL return so it persists for
118118
// the project's own CI even when this run skips the install.
119-
reconcilePreviewBridgeRegistry(cwd);
119+
reconcilePreviewBridgeRegistry(cwd, undefined, options?.packageManager);
120120

121121
// install dependencies on non-CI environment
122122
if (process.env.VP_SKIP_INSTALL) {

0 commit comments

Comments
 (0)