Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"picocolors": "^1.1.1"
},
"devDependencies": {
"@changesets/cli": "^2.27.0",
"@changesets/cli": "^2.30.0",
"eslint-plugin-react-hooks": "^7.0.1",
"oxfmt": "^0.32.0",
"oxlint": "^1.47.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/react-doctor/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ const resolveDiffMode = async (

const changedSourceFiles = filterSourceFiles(diffInfo.changedFiles);
if (changedSourceFiles.length === 0) return false;
if (shouldSkipPrompts) return true;
if (shouldSkipPrompts) return false;
if (isScoreOnly) return false;

const promptMessage = diffInfo.isCurrentChanges
Expand Down
3 changes: 1 addition & 2 deletions packages/react-doctor/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ export const GIT_LS_FILES_MAX_BUFFER_BYTES = 50 * 1024 * 1024;
// Use a conservative threshold to leave room for the executable path and quoting overhead.
export const SPAWN_ARGS_MAX_LENGTH_CHARS = 24_000;

export const OFFLINE_MESSAGE =
"You are offline, could not calculate score. Reconnect to calculate.";
export const OFFLINE_MESSAGE = "Score calculated locally (offline mode).";

export const DEFAULT_BRANCH_CANDIDATES = ["main", "master"];

Expand Down
9 changes: 9 additions & 0 deletions packages/react-doctor/src/plugin/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,15 @@ export const RAW_TEXT_PREVIEW_MAX_CHARS = 30;

export const REACT_NATIVE_TEXT_COMPONENTS = new Set(["Text", "TextInput"]);

export const REACT_NATIVE_TEXT_COMPONENT_SUFFIXES = new Set([
"Text",
"Title",
"Label",
"Heading",
"Caption",
"Subtitle",
]);

export const DEPRECATED_RN_MODULE_REPLACEMENTS: Record<string, string> = {
AsyncStorage: "@react-native-async-storage/async-storage",
Picker: "@react-native-picker/picker",
Expand Down
28 changes: 16 additions & 12 deletions packages/react-doctor/src/plugin/rules/nextjs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,24 +181,28 @@ export const nextjsMissingMetadata: Rule = {
}),
};

const isClientSideRedirect = (node: EsTreeNode): boolean => {
const describeClientSideNavigation = (node: EsTreeNode): string | null => {
if (node.type === "CallExpression" && node.callee?.type === "MemberExpression") {
const objectName = node.callee.object?.type === "Identifier" ? node.callee.object.name : null;
if (
objectName === "router" &&
(isMemberProperty(node.callee, "push") || isMemberProperty(node.callee, "replace"))
)
return true;
const methodName =
node.callee.property?.type === "Identifier" ? node.callee.property.name : null;
if (objectName === "router" && (methodName === "push" || methodName === "replace")) {
return `router.${methodName}() in useEffect — use redirect() from next/navigation or handle navigation in an event handler`;
}
}

if (node.type === "AssignmentExpression" && node.left?.type === "MemberExpression") {
const objectName = node.left.object?.type === "Identifier" ? node.left.object.name : null;
const propertyName = node.left.property?.type === "Identifier" ? node.left.property.name : null;
if (objectName === "window" && propertyName === "location") return true;
if (objectName === "location" && propertyName === "href") return true;
if (objectName === "window" && propertyName === "location") {
return "window.location assignment in useEffect — use redirect() from next/navigation or handle in middleware instead";
}
if (objectName === "location" && propertyName === "href") {
return "location.href assignment in useEffect — use redirect() from next/navigation or handle in middleware instead";
}
}

return false;
return null;
};

export const nextjsNoClientSideRedirect: Rule = {
Expand All @@ -209,11 +213,11 @@ export const nextjsNoClientSideRedirect: Rule = {
if (!callback) return;

walkAst(callback, (child: EsTreeNode) => {
if (isClientSideRedirect(child)) {
const navigationDescription = describeClientSideNavigation(child);
if (navigationDescription) {
context.report({
node: child,
message:
"Client-side redirect in useEffect — use redirect() from next/navigation or handle in middleware instead",
message: navigationDescription,
});
}
});
Expand Down
12 changes: 7 additions & 5 deletions packages/react-doctor/src/plugin/rules/react-native.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
RAW_TEXT_PREVIEW_MAX_CHARS,
REACT_NATIVE_LIST_COMPONENTS,
REACT_NATIVE_TEXT_COMPONENTS,
REACT_NATIVE_TEXT_COMPONENT_SUFFIXES,
} from "../constants.js";
import { hasDirective, isMemberProperty } from "../helpers.js";
import type { EsTreeNode, Rule, RuleContext } from "../types.js";
Expand Down Expand Up @@ -53,6 +54,11 @@ const getRawTextDescription = (child: EsTreeNode): string => {
return "text content";
};

const isTextHandlingComponent = (elementName: string): boolean => {
if (REACT_NATIVE_TEXT_COMPONENTS.has(elementName)) return true;
return [...REACT_NATIVE_TEXT_COMPONENT_SUFFIXES].some((suffix) => elementName.endsWith(suffix));
};

export const rnNoRawText: Rule = {
create: (context: RuleContext) => {
let isDomComponentFile = false;
Expand All @@ -65,11 +71,7 @@ export const rnNoRawText: Rule = {
if (isDomComponentFile) return;

const elementName = resolveJsxElementName(node.openingElement);
if (
elementName &&
(REACT_NATIVE_TEXT_COMPONENTS.has(elementName) || elementName.endsWith("Text"))
)
return;
if (elementName && isTextHandlingComponent(elementName)) return;

for (const child of node.children ?? []) {
if (!isRawTextContent(child)) continue;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-doctor/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export interface PackageJson {
dependencies?: Record<string, string>;
devDependencies?: Record<string, string>;
peerDependencies?: Record<string, string>;
workspaces?: string[] | { packages: string[] };
workspaces?: string[] | { packages?: string[]; catalog?: Record<string, string> };
}

export interface DependencyInfo {
Expand Down
155 changes: 152 additions & 3 deletions packages/react-doctor/src/utils/discover-project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,12 @@ const detectFramework = (dependencies: Record<string, string>): Framework => {

const isCatalogReference = (version: string): boolean => version.startsWith("catalog:");

const extractCatalogName = (version: string): string | null => {
if (!isCatalogReference(version)) return null;
const name = version.slice("catalog:".length).trim();
return name.length > 0 ? name : null;
};

const resolveVersionFromCatalog = (
catalog: Record<string, unknown>,
packageName: string,
Expand All @@ -144,7 +150,112 @@ const resolveVersionFromCatalog = (
return null;
};

const resolveCatalogVersion = (packageJson: PackageJson, packageName: string): string | null => {
interface CatalogCollection {
defaultCatalog: Record<string, string>;
namedCatalogs: Record<string, Record<string, string>>;
}

const parsePnpmWorkspaceCatalogs = (rootDirectory: string): CatalogCollection => {
const workspacePath = path.join(rootDirectory, "pnpm-workspace.yaml");
if (!isFile(workspacePath)) return { defaultCatalog: {}, namedCatalogs: {} };

const content = fs.readFileSync(workspacePath, "utf-8");
const defaultCatalog: Record<string, string> = {};
const namedCatalogs: Record<string, Record<string, string>> = {};

let currentSection: "none" | "catalog" | "catalogs" | "named-catalog" = "none";
let currentCatalogName = "";

for (const line of content.split("\n")) {
const trimmed = line.trim();
if (trimmed.length === 0 || trimmed.startsWith("#")) continue;

const indentLevel = line.search(/\S/);

if (indentLevel === 0 && trimmed === "catalog:") {
currentSection = "catalog";
continue;
}
if (indentLevel === 0 && trimmed === "catalogs:") {
currentSection = "catalogs";
continue;
}
if (indentLevel === 0) {
currentSection = "none";
continue;
}

if (currentSection === "catalog" && indentLevel > 0) {
const colonIndex = trimmed.indexOf(":");
if (colonIndex > 0) {
const key = trimmed.slice(0, colonIndex).trim().replace(/["']/g, "");
const value = trimmed
.slice(colonIndex + 1)
.trim()
.replace(/["']/g, "");
if (key && value) defaultCatalog[key] = value;
}
continue;
}

if (currentSection === "catalogs" && indentLevel > 0) {
if (trimmed.endsWith(":") && !trimmed.includes(" ")) {
currentCatalogName = trimmed.slice(0, -1).replace(/["']/g, "");
currentSection = "named-catalog";
namedCatalogs[currentCatalogName] = {};
continue;
}
}

if (currentSection === "named-catalog" && indentLevel > 0) {
if (indentLevel <= 2 && trimmed.endsWith(":") && !trimmed.includes(" ")) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard-coded indent breaks multi-catalog YAML parsing

Low Severity

The indentLevel <= 2 check in parsePnpmWorkspaceCatalogs hard-codes 2-space indentation for detecting sibling named catalog headers. With 4-space (or any >2) YAML indentation, the second and subsequent catalog names under catalogs: won't be recognized as new catalog sections — they'll be treated as key-value entries with empty values and silently skipped, causing their contents to be lost or misattributed to the previous catalog.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 1ea9414. Configure here.

currentCatalogName = trimmed.slice(0, -1).replace(/["']/g, "");
namedCatalogs[currentCatalogName] = {};
continue;
}
const colonIndex = trimmed.indexOf(":");
if (colonIndex > 0 && currentCatalogName) {
const key = trimmed.slice(0, colonIndex).trim().replace(/["']/g, "");
const value = trimmed
.slice(colonIndex + 1)
.trim()
.replace(/["']/g, "");
if (key && value) namedCatalogs[currentCatalogName][key] = value;
}
}
}

return { defaultCatalog, namedCatalogs };
};

const resolveCatalogVersionFromCollection = (
catalogs: CatalogCollection,
packageName: string,
catalogReference?: string | null,
): string | null => {
if (catalogReference) {
const namedCatalog = catalogs.namedCatalogs[catalogReference];
if (namedCatalog?.[packageName]) return namedCatalog[packageName];
}

if (catalogs.defaultCatalog[packageName]) return catalogs.defaultCatalog[packageName];

for (const namedCatalog of Object.values(catalogs.namedCatalogs)) {
if (namedCatalog[packageName]) return namedCatalog[packageName];
}

return null;
};

const resolveCatalogVersion = (
packageJson: PackageJson,
packageName: string,
rootDirectory?: string,
): string | null => {
const allDependencies = collectAllDependencies(packageJson);
const rawVersion = allDependencies[packageName];
const catalogName = rawVersion ? extractCatalogName(rawVersion) : null;

const raw = packageJson as Record<string, unknown>;

if (isPlainObject(raw.catalog)) {
Expand All @@ -153,6 +264,13 @@ const resolveCatalogVersion = (packageJson: PackageJson, packageName: string): s
}

if (isPlainObject(raw.catalogs)) {
if (catalogName && isPlainObject((raw.catalogs as Record<string, unknown>)[catalogName])) {
const version = resolveVersionFromCatalog(
(raw.catalogs as Record<string, unknown>)[catalogName] as Record<string, unknown>,
packageName,
);
if (version) return version;
}
for (const catalogEntries of Object.values(raw.catalogs)) {
if (isPlainObject(catalogEntries)) {
const version = resolveVersionFromCatalog(catalogEntries, packageName);
Expand All @@ -161,6 +279,21 @@ const resolveCatalogVersion = (packageJson: PackageJson, packageName: string): s
}
}

const workspaces = packageJson.workspaces;
if (workspaces && !Array.isArray(workspaces) && isPlainObject(workspaces.catalog)) {
const version = resolveVersionFromCatalog(
workspaces.catalog as Record<string, unknown>,
packageName,
);
if (version) return version;
}

if (rootDirectory) {
const pnpmCatalogs = parsePnpmWorkspaceCatalogs(rootDirectory);
const pnpmVersion = resolveCatalogVersionFromCollection(pnpmCatalogs, packageName, catalogName);
if (pnpmVersion) return pnpmVersion;
}

return null;
};

Expand Down Expand Up @@ -252,7 +385,7 @@ const findDependencyInfoFromMonorepoRoot = (directory: string): DependencyInfo =

const rootPackageJson = readPackageJson(monorepoPackageJsonPath);
const rootInfo = extractDependencyInfo(rootPackageJson);
const catalogVersion = resolveCatalogVersion(rootPackageJson, "react");
const catalogVersion = resolveCatalogVersion(rootPackageJson, "react", monorepoRoot);
const workspaceInfo = findReactInWorkspaces(monorepoRoot, rootPackageJson);

return {
Expand Down Expand Up @@ -342,6 +475,11 @@ export const listWorkspacePackages = (rootDirectory: string): WorkspacePackage[]

const packages: WorkspacePackage[] = [];

if (hasReactDependency(packageJson)) {
const rootName = packageJson.name ?? path.basename(rootDirectory);
packages.push({ name: rootName, directory: rootDirectory });
}

for (const pattern of patterns) {
const directories = resolveWorkspaceDirectories(rootDirectory, pattern);
for (const workspaceDirectory of directories) {
Expand Down Expand Up @@ -406,7 +544,18 @@ export const discoverProject = (directory: string): ProjectInfo => {
let { reactVersion, framework } = extractDependencyInfo(packageJson);

if (!reactVersion) {
reactVersion = resolveCatalogVersion(packageJson, "react");
reactVersion = resolveCatalogVersion(packageJson, "react", directory);
}

if (!reactVersion) {
const monorepoRoot = findMonorepoRoot(directory);
if (monorepoRoot) {
const monorepoPackageJsonPath = path.join(monorepoRoot, "package.json");
if (isFile(monorepoPackageJsonPath)) {
const rootPackageJson = readPackageJson(monorepoPackageJsonPath);
reactVersion = resolveCatalogVersion(rootPackageJson, "react", monorepoRoot);
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Named catalog reference lost during monorepo root resolution

Medium Severity

When a sub-package uses a named catalog reference like catalog:react_v19, resolveCatalogVersion correctly extracts the catalog name from the sub-package's dependencies on line 257. However, the pnpm-workspace.yaml isn't in the sub-package directory, so resolution fails. In the fallback on line 556, resolveCatalogVersion is called with the root's packageJson, which doesn't list react in its dependencies — so catalogName becomes null. resolveCatalogVersionFromCollection then iterates all named catalogs in arbitrary insertion order, potentially returning the wrong version when multiple named catalogs define the same package at different versions.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 1ea9414. Configure here.

}

if (!reactVersion || framework === "unknown") {
Expand Down
32 changes: 32 additions & 0 deletions packages/react-doctor/tests/discover-project.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,27 @@ describe("discoverProject", () => {

expect(() => discoverProject(projectDirectory)).toThrow("No package.json found");
});

it("resolves React version from pnpm workspace default catalog", () => {
const projectInfo = discoverProject(
path.join(FIXTURES_DIRECTORY, "pnpm-catalog-workspace", "packages", "ui"),
);
expect(projectInfo.reactVersion).toBe("^19.0.0");
});

it("resolves React version from pnpm workspace named catalog", () => {
const projectInfo = discoverProject(
path.join(FIXTURES_DIRECTORY, "pnpm-named-catalog", "packages", "app"),
);
expect(projectInfo.reactVersion).toBe("^19.0.0");
});

it("resolves React version from Bun workspace catalog", () => {
const projectInfo = discoverProject(
path.join(FIXTURES_DIRECTORY, "bun-catalog-workspace", "apps", "web"),
);
expect(projectInfo.reactVersion).toBe("^19.1.4");
});
});

describe("listWorkspacePackages", () => {
Expand All @@ -55,6 +76,17 @@ describe("listWorkspacePackages", () => {
expect(packageNames).toContain("ui");
expect(packages).toHaveLength(2);
});

it("includes monorepo root when it has a React dependency", () => {
const packages = listWorkspacePackages(
path.join(FIXTURES_DIRECTORY, "monorepo-with-root-react"),
);
const packageNames = packages.map((workspacePackage) => workspacePackage.name);

expect(packageNames).toContain("monorepo-root");
expect(packageNames).toContain("ui");
expect(packages).toHaveLength(2);
});
});

const tempDirectory = fs.mkdtempSync(path.join(os.tmpdir(), "react-doctor-discover-test-"));
Expand Down
Loading
Loading