fix(docs): stop Fern bundling commented-out component imports - #12753
fix(docs): stop Fern bundling commented-out component imports#12753dagil-nvidia wants to merge 3 commits into
Conversation
The "Preview or publish docs" job failed intermittently with: Failed to bundle third-party imports in fern/components/RecipeStyles.tsx: rolldown exited with code 127 ... sh: 1: rolldown: not found The named component rotated between RecipeStyles, ReferenceStyles and TerminalDemo run to run, and none of the three has an npm dependency. Fern collects a component's third-party dependencies with a plain-text regex over the whole source file, so it does not skip comments. Each of the three headers documented its own usage with a literal example statement naming "@/components/<Name>". Fern read that as a real dependency, and since "@/components" is outside its allowlist (react, react-dom, @mdx-js/react, next) it shelled out to `npx rolldown` to bundle it. The docs-website branch ships no package.json next to fern/, so that bundle could never resolve; it failed with exit 127 or a partially-unpacked npx cache, which is what made it look flaky. Document the specifier in prose instead, and add check_component_imports.py -- a port of Fern's own matcher, wired into pre-commit -- so a re-added example is caught before merge rather than on the publish. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Dan Gil <dagil@nvidia.com>
|
/ok to test bc08dba |
github-actions
Bot
added
fix
documentation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if package_root(specifier) in ALLOWLIST or specifier in seen: | ||
| continue | ||
| seen.add(specifier) | ||
| found.append((contents[: match.start()].count("\n") + 1, specifier)) |
There was a problem hiding this comment.
🟡 Docs check reports the wrong line number for offending import statements
The reported location is computed from the character just before the statement (match.start() at docs/fern/scripts/check_component_imports.py:98), which is the newline ending the previous line whenever a statement begins at the start of a line, so the tool points contributors at the line above the real problem.
Impact: Someone fixing a failing docs check is sent to the wrong line, wasting time and potentially editing unrelated content.
Why the leading-character alternation shifts the line count
The pattern begins with (?:^|[^\w.]) (docs/fern/scripts/check_component_imports.py:68). For a statement at column 0 on any line after the first, the regex engine reaches the preceding \n before it reaches the line-start anchor position, so [^\w.] consumes that newline and match.start() points at it. contents[: match.start()].count("\n") + 1 therefore excludes that newline and yields the previous line's number.
Verified: for "header\nimport { X } from \"@/components/X\";\n" the function returns line 1 instead of 2. Indented comment examples (like the ones this PR removed) happen to be preceded by a space on the same line, which is why the author's validation showed correct numbers.
Using the specifier group's start position instead avoids the shift.
| found.append((contents[: match.start()].count("\n") + 1, specifier)) | |
| start = match.start(1) if match.group(1) else match.start(2 if match.group(2) else 3) | |
| found.append((contents[:start].count("\n") + 1, specifier)) |
Was this helpful? React with 👍 or 👎 to provide feedback.
| The regex and allowlist below mirror the Fern CLI's own implementation | ||
| (cli.cjs, the helper behind "Failed to bundle third-party imports"); keep them | ||
| in sync when the pinned CLI in fern.config.json moves. |
There was a problem hiding this comment.
🔍 Guard regex is a hand-copy of Fern's matcher and can drift with the pinned CLI
The docstring states the regex and allowlist mirror the pinned Fern CLI's implementation and must be re-synced when fern.config.json moves. There is no automated check for that drift, so a CLI bump that widens the allowlist or changes the scan (e.g. to a real parser that skips comments) would leave this hook either over- or under-reporting. Worth a follow-up note in the docs release process.
Was this helpful? React with 👍 or 👎 to provide feedback.
![dynamo-review-agent[bot]](https://avatars.githubusercontent.com/u/201626793?s=60&v=4){kind=small}
WalkthroughThe pull request adds a Fern component import checker with self-tests, documents valid component usage, and adds pre-commit hooks for validation. ChangesFern component import validation
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/fern/scripts/check_component_imports.py (1)
110-119: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSpecify an explicit encoding for
read_text().
path.read_text()at line 111 uses the platform default encoding when noencodingargument is given. On a system where the locale-preferred encoding is not UTF-8 (for example Windows), reading a component source file containing non-ASCII characters can raiseUnicodeDecodeErrorand crash the pre-commit hook instead of reporting the specifier problem.Pass
encoding="utf-8"explicitly to make the read deterministic across platforms.🔧 Proposed fix
def check(path: Path) -> list[str]: - text = path.read_text() + text = path.read_text(encoding="utf-8") return [🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/fern/scripts/check_component_imports.py` around lines 110 - 119, Update the file read in check to pass encoding="utf-8" explicitly to path.read_text(), ensuring deterministic decoding across platforms while leaving the third_party processing unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/fern/scripts/check_component_imports.py`:
- Around line 110-119: Update the file read in check to pass encoding="utf-8"
explicitly to path.read_text(), ensuring deterministic decoding across platforms
while leaving the third_party processing unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5f5f0262-4a6c-4bd6-8607-925b1e925ca2
📒 Files selected for processing (5)
.pre-commit-config.yamldocs/fern/components/RecipeStyles.tsxdocs/fern/components/ReferenceStyles.tsxdocs/fern/components/TerminalDemo.tsxdocs/fern/scripts/check_component_imports.py
- Anchor reported line numbers on the matched specifier group rather than match.start(). The first alternation opens with (?:^|[^\w.]), which consumes the preceding newline for a statement at column 0, so the guard pointed at the line above. Only the import/export branch was affected; require() and import() carry no leading alternation. Self-test cases now assert the line number, including two column-0 regressions. - Fail a full scan when fern.config.json is bumped past the CLI release the matcher was ported from. The regex and allowlist are a hand-copy of Fern's bundler, and a CLI change that widens the allowlist or starts skipping comments would leave this check silently under-reporting -- which breaks the docs publish exactly the way the check exists to prevent. The pre-commit hook now watches fern.config.json so the bump trips it. Both raised by devin-ai-integration[bot] on #12753. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Dan Gil <dagil@nvidia.com>
|
/ok to test c52241b |
path.read_text() decodes with the locale-preferred encoding. 28 of the 33 scanned component sources are non-ASCII (em dashes in the doc headers), so under a non-UTF-8 locale the hook died with UnicodeDecodeError instead of reporting the specifier problem it exists to catch. Verified: under LC_ALL=C a bare read of RecipeStyles.tsx raises on byte 0xe2, and the explicit read succeeds. Raised by coderabbitai on #12753. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Dan Gil <dagil@nvidia.com>
|
/ok to test 51869fd |
|
Closing in favor of #12388, which fixes the same problem and does more. Both PRs independently rewrote the doc-comment import examples in `RecipeStyles.tsx`, `ReferenceStyles.tsx` and `TerminalDemo.tsx`, and both added a `docs/fern/scripts/check_component_imports.py` ported from the same fern-api 5.80.2 bundler. #12388 predates this one by eight days; I did not know it existed when I opened this. I verified the two are equivalent before choosing, rather than assuming the newer one was better:
#12388 additionally wires the hook into `.github/workflows/pre-merge.yml` and relocates the usage examples into `docs/fern/components/README.md`, where Fern's scan cannot reach them. Two things from this PR are genuinely better and I will port them onto #12388 rather than lose them: the finding output carries file:line instead of naming only the file, and the remediation text explains the CDN-at-runtime path for a real dependency. |
1 participant
Summary
The
Preview or publish docsjob has been failing onmainand on every docs-touching PR with:The named component rotated run to run —
RecipeStyles.tsx,ReferenceStyles.tsx,TerminalDemo.tsx— and none of the three has an npm dependency. That is the tell: the trigger was not a real import.Root cause. Fern collects a component's third-party dependencies with a plain-text regex over the whole source file. It does not parse the source, so it does not skip comments. Each of the three headers documented its own usage with a literal example statement naming
@/components/<Name>. Fern read that comment as a real dependency;@/componentsis outside its allowlist (react,react-dom,@mdx-js/react,next), so it shelled out tonpx rolldownto bundle it.The
docs-websitebranch ships nopackage.jsonnext tofern/, so that bundle could never resolve. It surfaced two ways, which is what made it look flaky rather than broken:127—rolldown: not found1—ERR_MODULE_NOT_FOUNDon a half-unpacked~/.npm/_npx/…/rolldown/dist/shared/binding-*.mjsFern bundles the components concurrently, so several
npxinvocations race on one shared cache directory. A retry that happens to win the race goes green, which is why re-runs sometimes "fixed" it.Fix. Document the specifier in prose in all three headers instead of writing a literal statement, and add
docs/fern/scripts/check_component_imports.py— a port of Fern's own matcher (regex, allowlist, and package-root reduction taken from the pinned CLI) — wired into pre-commit so a re-added example is caught before merge instead of on the publish.No page content, CSS, or component behaviour changes; the edits are comment-only.
Validation
docs/fern/componentsatmain: flagged exactlyRecipeStyles.tsx:17,ReferenceStyles.tsx:23,TerminalDemo.tsx:15,28— the same three files CI named, and no others.checked 33 component source(s): no unbundleable specifiers.fern-docs.yml, therm -rf+cp -rintodocs-checkout/fern/components) against a realorigin/docs-websiteworktree and re-ran the check on the composed tree — clean, 33/33. Confirmeddocs-websitecarries exactly these 33 files, so the branch copy cannot drift.RecipeStyles.tsx, and 0 on the fixed tree.--testself-test covers 12 cases, including allowlisted, scoped-allowlisted, subpath, relative, dynamicimport(),require(), re-export, the commented-example regression, and prose that merely contains the word.pre-commit run --files …green on all five changed files.Not fixed here, and tracked separately:
mainis also red on the laterVerify published pages carry their component CSSstep, which 404s on/dynamo/dev/recipes/feature-benchmarks/llama-3-3-70b-topology. That is a different failure in a different step —Publish Docsitself succeeds onmain.🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Quality Improvements