fix(packages/core): emit sidebar keys for orphaned workspace children#74
fix(packages/core): emit sidebar keys for orphaned workspace children#74zrosenbauer merged 5 commits intomainfrom
Conversation
When workspace items (apps/packages) use paths outside the parent prefix (e.g. `/libs/ai` under parent `/packages`), Rspress prefix matching could not find a sidebar key and the sidebar silently disappeared. Extra sidebar keys are now emitted for each orphaned child link so pages at those paths resolve to the correct sidebar. Co-Authored-By: Claude <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 1dc727d The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughA Changesets entry was added to publish a patch for 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/sync/sidebar/multi.test.ts`:
- Line 187: The test uses the long-form generic array type for the sidebar
variable; change the type annotation on sidebar (const sidebar =
result['/libs/ai/'] as Array<{ text: string; link: string }>) to the shorthand
T[] form (i.e., use { text: string; link: string }[]), updating the cast on
result['/libs/ai/'] to the shorter array type.
- Around line 169-171: The test currently mutates via
[...lengths].sort(...)—replace that with an immutable sort using
Array.prototype.toSorted on the lengths array (e.g., use lengths.toSorted((a,b)
=> b - a)) and update the `sorted` assignment accordingly so `lengths` is not
mutated; refer to the `lengths` and `sorted` variables in multi.test.ts and keep
the existing expect(lengths).toEqual(sorted) assertion semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0734b4d2-952e-4da5-9e59-1c0d6717b4aa
📒 Files selected for processing (3)
.changeset/fix-orphaned-workspace-sidebar.mdpackages/core/src/sync/sidebar/multi.test.tspackages/core/src/sync/sidebar/multi.ts
Use toSorted() instead of spread+sort and T[] syntax instead of Array<T>. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/sync/sidebar/multi.test.ts`:
- Around line 84-85: Change the two assertions to check referential equality
rather than deep equality: assert that result['/libs/ai/'] and
result['/libs/db/'] are the exact same array instance as result['/packages/']
(use toBe instead of toEqual). This ensures the test verifies the orphaned keys
in the multi.ts implementation (the code that assigns the same sidebarItems
array reference) still returns the same array object, not just an equal copy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 71b7ab90-9e2a-446d-b368-999099b15498
📒 Files selected for processing (1)
packages/core/src/sync/sidebar/multi.test.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Zac Rosenbauer <zac@joggr.io>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/sync/sidebar/multi.test.ts`:
- Around line 186-190: The test dereferences sidebar[0] without asserting the
array is non-empty; add an explicit assertion that the computed sidebar from
buildMultiSidebar(entries, []) is not empty (e.g.,
expect(sidebar.length).toBeGreaterThan(0) or
expect(sidebar).not.toHaveLength(0)) immediately before the existing check so
failures clearly show missing/empty sidebar content rather than an out-of-bounds
access on sidebar[0].
- Around line 69-86: Update the test in multi.test.ts that calls
buildMultiSidebar to also assert that non-trailing orphaned keys reuse the same
sidebar reference: after the existing expect checks for result['/libs/ai/'] and
result['/libs/db/'] equaling result['/packages/'], add asserts that
result['/libs/ai'] and result['/libs/db'] are strictly equal to
result['/packages/'] as well so both trailing-slash and non-trailing-slash
orphan keys reuse the same sidebar object produced by buildMultiSidebar.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: dccf606c-9409-4901-96e8-bc67bc20e467
📒 Files selected for processing (1)
packages/core/src/sync/sidebar/multi.test.ts
- Assert sidebar is non-empty before accessing sidebar[0] - Assert both trailing-slash and non-trailing-slash orphaned keys match the parent sidebar content Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/sync/sidebar/multi.test.ts`:
- Around line 188-191: The test dereferences result['/libs/ai/'] into sidebar
and immediately accesses sidebar.length which can throw if the key is missing;
update the test around buildMultiSidebar(entries, [])/result to first assert the
key exists (e.g. expect(result['/libs/ai/']).toBeDefined() or expect('/libs/ai/'
in result).then narrow to the typed sidebar variable before asserting
expect(sidebar.length).toBeGreaterThan(0) so the failure is a clear assertion
rather than a runtime throw.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 544e15cd-4b74-4a1a-94d5-ea2e626894df
📒 Files selected for processing (1)
packages/core/src/sync/sidebar/multi.test.ts
1 participant
Summary
apps/packages) use paths outside the parent prefix (e.g./libs/aiunder parent/packages)Changes
packages/core/src/sync/sidebar/multi.ts— AddedcollectOrphanedChildLinks()helper and emit orphaned keys inbuildMultiSidebar()packages/core/src/sync/sidebar/multi.test.ts— New test file with 9 tests covering orphaned keys, aligned paths, mixed children, and edge casesTesting