Repository navigation
merge: reconcile upstream through d0bb9be5 - #501
Merged
Merged
Conversation
…mport (colbymchenry#2558) A lazily loaded folder whose index passes on a page it imports linked nothing: DigiShop.Frontend's `src/layout/index.ts` does `import { UserLayout } from './UserLayout'; export { UserLayout }`, and its routes load `await import('@/layout')`. exportedComponent, the barrel walk behind lazy routes, React.lazy values and lazy layouts (colbymchenry#2436, colbymchenry#2452, colbymchenry#2480), read the export clause as naming a declaration in the barrel, which has none: the name is an import binding. `import Login from './Login'; export default Login` and `export { Page as Component }` of an imported Page failed the same way, so all 27 lazy route refs in DigiShop's App.tsx stayed failed. A binding the module exports but does not declare is now followed through the module's import of it, a default import as `default` and a named one by the name it imports, with each file read added to `reads` so lazyModules still redraws the route. A namespace import or a package's binding links nothing. `export default withAuth(Page)` hands over a call, whose first identifier ownExport reads; that was harmless while only same-file declarations counted, so a default is followed only when the statement is the binding itself (exportsBinding), or the route would link the HOC. colbymchenry#2451 (open) adds the same step to the general import walk; colbymchenry#2420 (open) carried it in a lazyRouteComponent rewrite that exportedComponent superseded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…olbymchenry#2560) A lazy React Router route whose module is a barrel, and whose page sits in a folder below the barrel's, stayed unlinked after `codegraph sync` added the page or gave it its export, though a fresh index linked it. In DigiShop.Frontend, `@/pages` is `src/pages/index.ts`, which imports `./Home`, `src/pages/Home/index.tsx`, and exports it again. While the page is missing, the route's reference fails and waits under the key of the module it names, `module:pages` (colbymchenry#2422). The retry looks failed references up by the keys of the changed files, and a file in `pages/Home/` gives only `module:Home` and its own stem. The limit, which the `export *` barrels of colbymchenry#2436 share, was recorded with colbymchenry#2558. After the failed-ref retry, which makes the barrel's import edge into an added file, sync now walks up the import edges from each file it added, or gave a definition, to the barrels that pass it on: `export … from`, or an import exported again (`import { X } from './x'; export { X }`, `export default X`), as deep as the resolver chases a name. It re-opens, for the sweep, the failed references waiting under those barrels' keys. Only keys a failed reference waits under are looked up, in one DISTINCT over the failed rows parked under module keys, so a sync with none reads nothing, and a body-only edit seeds nothing. The ImportReader that the added-files reopen (colbymchenry#2512) uses is shared, and gains exportedBindings, behind a regex pre-check, since most importers export nothing. defaultExportBinding moves to alias-binding.ts for this walk and for react.ts's exportsBinding. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… JSX renders (colbymchenry#2420) A PascalCase name that a .tsx/.jsx file declares as a module-level value is that declaration wherever the file uses it: a JSX tag, a route's element, a layout or a TSX type. Its initializer decides what it renders, in order: - the component of the module its lazy loader loads, or the export the loader picks, in either branch of a conditional; - the function it wraps (`observer(function X() {…})`, `forwardRef(Inner) as {…}`); - else the declaration itself. Before, the lookup by kind skipped the value and took a lone same-named component anywhere, often in another app of the repository. - Pattern 1 asks declaredComponent about every PascalCase reference, not only route references. layoutComponent and the jsx-render synthesizer's jsxChild ask it too. - valueLoader reads a value's loader with loaderValue (colbymchenry#2480). - defaultExportedName reads `export default observer(Login)`. - lazyValueModules records the modules a file's lazy values load as synthesis inputs, so a sync that changes one of them redraws the tag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings in upstream colbymchenry#2558 (a lazy route follows a barrel that re-exports an import), colbymchenry#2560 (sync re-opens a lazy route when a page appears below its barrel) and colbymchenry#2420 (a component a file declares as a value is what its JSX renders). Fork-side choices: - The CodeGraph class lives in src/codegraph.ts, so upstream's sync-path call to reopenReferencesThroughBarrels goes there; src/index.ts stays the fork's. - extractLocalExportAliases and defaultExportBinding are not brought back. The import reader's exportedBindings, exportedComponent's local export and its "default export is this binding" check read the kernel's binding rows (exportedAs on import and declaration rows). A nodeless `default` row, which `export default observer(Page)` leaves, falls through to the default-export reader, as upstream does. - declaredValue looks the name up through getNodesByName, because the fork's context has no getNodesInFileNamed. - collectInputs and lazyValueModules are dropped: the fork re-runs synthesis in full on any sync that changes a file and keeps no synthesis-input list. - jsx-child-disambiguation.test.ts: one fixture writes `Sign in: <LoginPage />` in place of `Sign in <LoginPage />`, because the fork's opensTag reads a word before `<` as a type or a comparison. The assertion is unchanged. - framework-coverage.md: the React Router row keeps the fork's text and adds upstream's evidence, marked upstream. Trap 19 describes the fork's full resynthesis in place of lazyValueModules. The row's verified date stays at 2026-10-07 because the fork has not re-verified it. - README names upstream main through d0bb9be.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly identifies the primary change: reconciling the fork with upstream through commit d0bb9be. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Suppressions Explained | |
PASS. The pull-request diff adds no lint, type-check, or compiler suppression directive, and it adds no lint or TypeScript configuration ignore entry. The only failure-related test marker is `it.fails… |
| User-Visible Changes Documented | |
The diff does not add, remove, or rename a CLI command or flag, MCP tool argument, supported language or framework, agent target, or config key. The React and TypeScript/JavaScript changes extend exis… |
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 13 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
…scope Trap 19 and note the type/value namespace limit The reworded fixture hid a fork limit (opensTag reads a word before '<' as a type or comparison). The reworded case stays, and upstream's own text is added as an it.fails case so the limit stays visible. Trap 19 now says Pattern 1 covers PascalCase references in .tsx/.jsx files and names the type/value namespace limit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fork/consolidatednow contains upstreammainthroughd0bb9be5(three commits), so a React Router lazy route follows a barrel that re-exports an import (colbymchenry#2558), relinks a page that appears below its barrel on sync (colbymchenry#2560), and a component a file declares as a value is what its JSX renders (colbymchenry#2420).Conflicts. Seven files conflicted, because upstream's fixes were written against the regex-based JavaScript resolver and the fork reads the kernel's binding rows instead (
91902770). Nothing brings the regexes back.src/index.tssrc/codegraph.ts, where the class now lives;createImportReadertakes(root, queries)on the fork andchangedFilePathsmay be absent, so both are handled.src/resolution/alias-binding.tsextractLocalExportAliasesanddefaultExportBindingstay deleted.src/resolution/module-importers.tsEXPORTS_A_BINDINGregex is replaced byexportedBindings, built from the file's top-level binding rows that carryexportedAs(the nodelessdefaultrow ofexport default withAuth(Page)is skipped). Upstream'sreopenReferencesThroughBarrelsandpassesOncame in as written.src/resolution/frameworks/react.tsdeclaredComponent,valueLoader,loaderOf,wrappedComponent), withexportedComponentandexportsBindingreading binding rows anddeclaredValueusinggetNodesByName(the fork has nogetNodesInFileNamed).src/resolution/callback-synthesizer.tsdeclaredComponent. Upstream's input-file list (collectInputs,lazyValueModules) is not taken: the fork re-runs synthesis in full on a sync that changes a file.__tests__/react-router.test.ts,docs/design/framework-coverage.mdChecks, on this head. Upstream's five new or changed test files: 123 of 123 pass. Full suite 723 files / 9,087 tests pass (39 skipped);
tsc --noEmitandcheck:agent-docsclean; golden dumps unchanged. Precision gate (eval:precision): vite absent 6/6 and present 2/2 held, vitest absent 1/1 held.README. The "About this fork" merge point now names
d0bb9be5. Rows checked and left as they were: fork-vs-upstream benchmark, React Router,getHref, framework mode, Solid, inferred links kept current on sync.One fixture edited. In
__tests__/jsx-child-disambiguation.test.tsthe textSign in <LoginPage />becameSign in: <LoginPage />; the assertion is unchanged. The fork's tag detector (opensTag) reads a word before<as a type or comparison, so it draws no render edge there where upstream does. That is older than this merge and is not changed here.Not covered. Indexing time with the new
declaredComponentlookup (one cachedgetNodesByNameper PascalCase reference in.tsx/.jsx) was not measured. The merge commit keeps upstream'sd0bb9be5as a parent, so land it as a merge commit. Promoting the deployed build is a separate step.Summary by cubic
Reconciles
fork/consolidatedwith upstreammainthroughd0bb9be5, adopting three fixes: lazy React Router routes follow a barrel that re-exports an import, sync relinks a route when its page appears below its barrel, and a component a file declares as a value is what its JSX renders. Upstream's five test files pass 123/123, the full suite is clean (723 files, 9,087 tests, 39 skipped), and golden dumps are unchanged.Conflict resolutions. The seven conflicted files share one choice: upstream's regex-based JavaScript resolver is not brought back; its fixes were ported to read the kernel's binding rows instead.
src/codegraph.tskeeps the fork's version; the sync-pathreopenReferencesThroughBarrelscall moved here, handling the fork'screateImportReader(root, queries)signature and a possibly absentchangedFilePaths.module-importers.tsreplacesEXPORTS_A_BINDINGwithexportedBindings, built from top-level binding rows carryingexportedAs;extractLocalExportAliasesanddefaultExportBindingstay deleted.react.tsreads exports from binding rows and looks up declared values throughgetNodesByName(the fork has nogetNodesInFileNamed).callback-synthesizer.tsdrops upstream'scollectInputs/lazyValueModulesinput list: the fork re-runs synthesis in full on any sync that changes a file.__tests__/react-router.test.tsanddocs/design/framework-coverage.mdkeep both sides.One fixture in
jsx-child-disambiguation.test.tswas edited,Sign in: <LoginPage />forSign in <LoginPage />, because the fork's tag detector reads a word before<as a type or comparison and draws no render edge there; upstream's original text is kept as anit.failscase so the limit stays visible, and the assertion is unchanged. Trap 19 inframework-coverage.mdscopes Pattern 1 to PascalCase references in.tsx/.jsxfiles and notes the type/value namespace limit. The README's merge point now namesd0bb9be5. Indexing cost of the newdeclaredComponentlookup was not measured, and promoting the deployed build is a separate step.Written for commit 5b73098. Summary will update on new commits.
Summary by CodeRabbit