From d25e10b9c2639c4c570e9ecbf89979e7e559ae96 Mon Sep 17 00:00:00 2001 From: Alaister Young Date: Mon, 27 Jul 2026 14:24:45 +0800 Subject: [PATCH] fix(ui-patterns): fix admonition self-import + add case-sensitivity guard (#48344) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the TanStack Studio app failing to load locally, and adds a CI guard so the same class of bug can't come back. `packages/ui-patterns/src/admonition.tsx` is a back-compat shim containing `export * from './Admonition'`. On a case-insensitive filesystem (macOS, Windows) the resolver tries `./Admonition.tsx` before the directory index — and that's the same file. The shim re-exported itself and exported nothing, so every consumer of `ui-patterns/admonition` blew up with `does not provide an export named 'Admonition'`, plus knock-on Vite dep-optimizer errors about missing chunks. It works on Linux, so typecheck, lint, build and tests all pass on CI. This only reproduces on dev machines. **Changed:** - `admonition.tsx` now points at `./Admonition/index` explicitly, so the specifier can't resolve back to itself **Added:** - `scripts/check-case-hazards.mjs` — dependency-free, two textual checks so they fire on Linux CI: - **Self-resolving imports**: for `dir/X.tsx`, flags any extension-less relative specifier resolving to `dir/X` case-insensitively - **Case-colliding paths**: tracked paths (files and directory prefixes) equal when lowercased, which can't coexist in a case-insensitive checkout - `pnpm test:case-hazards`, plus a step in `typecheck.yml` after `setup-node` but before `pnpm install` — no deps needed, fails fast Scoped check 1 to genuine self-imports rather than all case-insensitive file/directory ambiguity. The broader rule lights up ~45 legitimate routing pairs (`_app.tsx` + `_app/`, `changelog.tsx` + `changelog/`) and would get switched off within a week. This version has zero false positives on master today. ## Follow-up (not in this PR) The underlying duplication is still there: `src/admonition.tsx` and `src/Admonition/` both exist, and the ~14 internal `'../Admonition'` imports inside the package resolve through the shim on macOS but through the directory on Linux. Two resolution paths for one module is exactly what produced this. Real fix is to collapse it. Current counts: **246** files import `ui-patterns/admonition`, **0** import `ui-patterns/Admonition`. So either rename the directory to lowercase and delete the shim (zero import churn, but one lowercase dir among ~50 PascalCase siblings), or codemod the 246 imports to PascalCase to match the package convention. I'd lean to the codemod, on a day it won't conflict with in-flight branches. ## To test - `pnpm test:case-hazards` on master → passes, ~16.5k files checked - Revert `admonition.tsx` to `export * from './Admonition'` and re-run → fails with the offending file and the suggested fix - Confirm the fixed form `'./Admonition/index'` is *not* flagged - With the fix in place: `rm -rf apps/studio/node_modules/.vite`, then `STUDIO_FRAMEWORK=tanstack pnpm dev:studio` → app loads, no `Pre-transform error` and no missing-export error in the console ## Summary by CodeRabbit * **Bug Fixes** * Improved detection of file-path casing issues that could cause failures on case-insensitive systems. * Corrected a module re-export to ensure the intended UI component is exposed consistently. * **Tests** * Added a dedicated case-sensitivity hazard check. * Integrated the check into the type-check workflow for earlier issue detection. --------- Co-authored-by: Alaister Young <10985857+alaister@users.noreply.github.com> --- .github/workflows/typecheck.yml | 6 + package.json | 1 + packages/ui-patterns/src/admonition.tsx | 5 +- scripts/check-case-hazards.mjs | 170 ++++++++++++++++++++++++ 4 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 scripts/check-case-hazards.mjs diff --git a/.github/workflows/typecheck.yml b/.github/workflows/typecheck.yml index fc7643f5809..41817e88c20 100644 --- a/.github/workflows/typecheck.yml +++ b/.github/workflows/typecheck.yml @@ -37,6 +37,12 @@ jobs: node-version-file: '.nvmrc' cache: 'pnpm' + # Needs no dependencies, so it runs before install and fails fast. Catches a + # class of bug that only breaks on case-insensitive filesystems (macOS and + # Windows dev machines) and is therefore invisible to typecheck/lint on CI. + - name: Check for case-sensitivity hazards + run: node scripts/check-case-hazards.mjs + - name: Install deps run: pnpm install --frozen-lockfile diff --git a/package.json b/package.json index 1040329e8f9..0caaeb35052 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "dev:design-system": "turbo run dev --filter=design-system --parallel", "lint": "turbo run lint", "typecheck": "turbo --continue typecheck", + "test:case-hazards": "node scripts/check-case-hazards.mjs", "test:prettier": "SORT_IMPORTS=false prettier --config prettier.config.mjs --cache --check '{apps,packages,blocks,examples,i18n}/**/*.{js,jsx,ts,tsx,css,md,mdx,json}'", "format": "SORT_IMPORTS=false prettier --config prettier.config.mjs --cache --write '{apps,packages,blocks,examples,i18n}/**/*.{js,jsx,ts,tsx,css,md,mdx,json}'", "test:docs": "turbo run test --filter=docs", diff --git a/packages/ui-patterns/src/admonition.tsx b/packages/ui-patterns/src/admonition.tsx index 6163e4fb671..11ef9105ab8 100644 --- a/packages/ui-patterns/src/admonition.tsx +++ b/packages/ui-patterns/src/admonition.tsx @@ -1 +1,4 @@ -export * from './Admonition' +// Must point at ./Admonition/index explicitly: on case-insensitive filesystems +// (macOS) a bare './Admonition' resolves to './Admonition.tsx', which is this +// same file, making the re-export a no-op that exports nothing. +export * from './Admonition/index' diff --git a/scripts/check-case-hazards.mjs b/scripts/check-case-hazards.mjs new file mode 100644 index 00000000000..0d25d761cc2 --- /dev/null +++ b/scripts/check-case-hazards.mjs @@ -0,0 +1,170 @@ +#!/usr/bin/env node +/** + * Guards against filesystem case-sensitivity hazards that CI cannot otherwise see. + * + * CI runs on Linux (case-sensitive); most contributors are on macOS or Windows + * (case-insensitive). Code can therefore be correct on CI and broken on every + * dev machine, with typecheck, lint, build and tests all passing. + * + * Two checks, both purely textual so they fire regardless of the host filesystem: + * + * 1. Self-resolving imports. A specifier like `./Admonition` inside a file + * named `admonition.tsx` resolves to the *directory* on Linux, but on a + * case-insensitive filesystem the bundler tries `./Admonition.tsx` first + * and lands back on the importing file. The module then re-exports itself + * and silently exports nothing. Write `./Admonition/index` instead. + * + * 2. Paths colliding only by case. Two tracked paths equal when lowercased + * cannot both exist in a case-insensitive checkout; one silently clobbers + * the other on clone. + * + * Usage: node scripts/check-case-hazards.mjs + */ +import { execFileSync } from 'node:child_process' +import fs from 'node:fs' +import path from 'node:path' + +const SOURCE_EXTENSIONS = ['.ts', '.tsx', '.js', '.jsx', '.mjs', '.cjs'] + +// Directories whose contents are generated or vendored — we don't control their +// naming and can't fix violations in them. +const IGNORED_SEGMENTS = ['node_modules', '__generated__', '.generated'] + +/** Every relative specifier form we need to catch: static, dynamic, and require. */ +const SPECIFIER_PATTERNS = [ + /\bfrom\s*['"]([^'"]+)['"]/g, // import x from '…' / export * from '…' + /\bimport\s*\(\s*['"]([^'"]+)['"]/g, // import('…') + /\bimport\s+['"]([^'"]+)['"]/g, // import '…' + /\brequire\s*\(\s*['"]([^'"]+)['"]/g, // require('…') +] + +function listTrackedFiles() { + const stdout = execFileSync('git', ['ls-files', '-z'], { + encoding: 'utf8', + maxBuffer: 64 * 1024 * 1024, + }) + return stdout.split('\0').filter(Boolean) +} + +function isIgnored(file) { + return file.split('/').some((segment) => IGNORED_SEGMENTS.includes(segment)) +} + +function stripExtension(basename) { + const extension = SOURCE_EXTENSIONS.find((candidate) => basename.endsWith(candidate)) + return extension ? basename.slice(0, -extension.length) : basename +} + +function extractRelativeSpecifiers(source) { + const specifiers = new Set() + + for (const pattern of SPECIFIER_PATTERNS) { + // Patterns carry /g, so reset lastIndex before reusing them across files. + pattern.lastIndex = 0 + let match + while ((match = pattern.exec(source)) !== null) { + const specifier = match[1] + if (specifier.startsWith('./') || specifier.startsWith('../')) { + specifiers.add(specifier) + } + } + } + + return [...specifiers] +} + +/** + * Flags a specifier that resolves back to its own importing file once the + * filesystem stops distinguishing case. Only specifiers without a file + * extension are at risk: given an explicit extension the resolver never appends + * one, so it cannot land on a same-named sibling. + */ +function findSelfResolvingImports(files) { + const violations = [] + + for (const file of files) { + if (!SOURCE_EXTENSIONS.some((extension) => file.endsWith(extension))) continue + + const source = fs.readFileSync(file, 'utf8') + const directory = path.dirname(file) + const ownName = stripExtension(path.basename(file)).toLowerCase() + + for (const specifier of extractRelativeSpecifiers(source)) { + if (SOURCE_EXTENSIONS.some((extension) => specifier.endsWith(extension))) continue + + const target = path.resolve(directory, specifier) + const isSameDirectory = + path.dirname(target).toLowerCase() === path.resolve(directory).toLowerCase() + const isSameName = path.basename(target).toLowerCase() === ownName + + if (isSameDirectory && isSameName) { + violations.push({ file, specifier }) + } + } + } + + return violations +} + +/** + * Flags tracked paths — files and the directories leading to them — that differ + * only by case. Git stores both happily; a case-insensitive checkout cannot. + */ +function findCaseCollisions(files) { + const seen = new Map() + const collisions = new Map() + + const record = (candidate) => { + const key = candidate.toLowerCase() + const existing = seen.get(key) + + if (existing === undefined) { + seen.set(key, candidate) + return + } + if (existing === candidate) return + + if (!collisions.has(key)) collisions.set(key, new Set([existing])) + collisions.get(key).add(candidate) + } + + for (const file of files) { + record(file) + + const segments = file.split('/') + for (let i = 1; i < segments.length; i++) { + record(segments.slice(0, i).join('/')) + } + } + + return [...collisions.values()].map((paths) => [...paths].sort()) +} + +const files = listTrackedFiles().filter((file) => !isIgnored(file)) + +const selfResolvingImports = findSelfResolvingImports(files) +const caseCollisions = findCaseCollisions(files) + +if (selfResolvingImports.length === 0 && caseCollisions.length === 0) { + console.log(`✅ No case-sensitivity hazards found (${files.length} tracked files checked).`) + process.exit(0) +} + +if (selfResolvingImports.length > 0) { + console.error('\n❌ Imports that resolve to their own file on a case-insensitive filesystem:\n') + for (const { file, specifier } of selfResolvingImports) { + console.error(` ${file}`) + console.error(` imports '${specifier}', which matches this file's own name.`) + console.error(` Point at the directory index explicitly, e.g. '${specifier}/index'.\n`) + } +} + +if (caseCollisions.length > 0) { + console.error('\n❌ Tracked paths that differ only by case:\n') + for (const paths of caseCollisions) { + console.error(` ${paths.join('\n ')}\n`) + } + console.error(' These cannot coexist in a case-insensitive checkout. Rename one.\n') +} + +process.exit(1)