fix(rest): drop a nav group DECLARED empty, not just one the gate emptied (#7380) - #7460
Merged
Merged
Conversation
…tied (#7380) `filterNav`'s docblock has promised "Drop empty groups so the sidebar doesn't render a label with nothing under it" since #4651, but the guard in front of that branch was `Array.isArray(e.children) && e.children.length > 0`. A group authored `children: []` therefore never entered the branch that OWNS the rule — it fell through the else and shipped in `GET /meta/app` as a bare label. The one shape the sentence most obviously covers was the one shape it could not reach. The judgement is now on what SURVIVES rather than on how the entry got there: a `type: 'group'` with no surviving children is dropped whether it BECAME empty (children gated away) or STARTED empty (`children: []`). Nesting composes, and a group carrying no `children` key at all — unreachable through the spec, but reachable at runtime because this filter reads untyped documents off the metadata store — is the same dead label and drops too. Contribution slots are the shape this actually shipped. `setup.app.ts` is authored entirely out of it: nine group anchors with `children: []`, filled on read by `Registry.applyNavContributions` (ADR-0029 D7). That merge runs in the protocol layer BEFORE this filter, so a filled slot arrives with children and survives while an unfilled one arrives `[]` and is now dropped — exactly the "a disabled capability contributes nothing and its slot stays empty" case `setup.app.ts` documents. The rule is `type: 'group'` and nothing else. An `object` entry navigates on its own `objectName`, so emptiness says nothing about whether to serve it; non-group entries keep today's behaviour exactly. A group cannot be a target — `GroupNavItemSchema` is a `strictObject` declaring no target key and rejecting any, and reads "Does not perform navigation itself". Measured before the change: of the 16 childless `type: 'group'` entries across the shipped apps and the examples, zero carry a target key, so the drop is unconditional. Because `areas[].navigation` runs through the same `filterNav`, an area holding only childless groups now empties and is dropped by the existing area rule; an area authored `navigation: []` still passes through, and that divergence is documented at `filterAreas`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNJY6He34CWyS8VDBtxVM9
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
size/m
documentation
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 9 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 3 release-owned page(s) also reference the affected code. These are read-only:
|
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.
Fixes #7380
filterNav's docblock has promised "Drop empty groups so the sidebar doesn't render a label with nothing under it" since #4651, but the guard in front of that branch wasArray.isArray(e.children) && e.children.length > 0. A group authoredchildren: []therefore never entered the branch that owns the rule — it fell through theelseand shipped inGET /meta/appas a bare label. The one shape the sentence most obviously covers was the one shape it could not reach.The rule now
The judgement is on what survives rather than on how the entry got there: a
type: 'group'with no surviving children is dropped whether it became empty (children gated away) or started empty (children: []). Nesting composes, and a group carrying nochildrenkey at all — unreachable through the spec, but reachable at runtime because this filter reads untyped documents off the metadata store — is the same dead label and drops too.Contribution slots are the shape this actually shipped.
setup.app.tsis authored entirely out of it: nine group anchors withchildren: [], filled on read byRegistry.applyNavContributions(ADR-0029 D7). That merge runs in the protocol layer before this filter, so a filled slot arrives with children and survives while an unfilled one arrives[]and is now dropped — exactly the "a disabled capability contributes nothing and its slot stays empty" casesetup.app.tsdocuments.The open sub-question, answered by measurement
The card asked whether a childless group is ever legitimately a navigation target on its own. It is not, and this was measured rather than assumed:
GroupNavItemSchemais astrictObjectdeclaring no target key and rejecting any, and reads "Does not perform navigation itself".type: 'group'entries across the shipped apps and the examples, zero carry a target key.So the drop is unconditional. The rule is
type: 'group'and nothing else — anobjectentry navigates on its ownobjectName, so emptiness says nothing about whether to serve it, and non-group entries keep today's behaviour exactly.Because
areas[].navigationruns through the samefilterNav, an area holding only childless groups now empties and is dropped by the existing area rule; an area authorednavigation: []still passes through, and that divergence is documented atfilterAreas.Scope
packages/rest/src/rest-server.ts(filterNavregion only) +rest.test.ts+ changeset. No response-envelope site is touched, so #7295's ratchet baseline is unaffected.Implemented by a dispatched dev agent in its own container; PR opened by the PM seat because the container had git push but no GitHub API access. Full rationale is preserved verbatim in the commit message of
e99824010.Generated by Claude Code