fix: resolve pinned nav rows from the bundled manifest instead of refetching it - #6437
Merged
Conversation
…etching it (#6362) Layout has imported NAV_COMMANDS statically since #5053, but it still fetched GET /api/palette/manifest on every mount to resolve the Pinned and Recent rows whose paths have no sidebar presentation (/wiki/log, /goals/tree, and the previousPaths migrations). Those rows rendered nothing until the request landed, then popped in — and stayed missing for the whole session whenever the request failed, which a password-gated install with an expired session hits routinely. Derive manifestEntryByPath from the manifest the bundle already carries (reusing the module-level commandByPath map), pass NAV_COMMANDS to migrateLegacyNavPath, and drop the api import that existed only for the fetch. The endpoint itself is unchanged: CmdK and the voice widget still need the server to hydrate its actions half. Layout.test.jsx no longer mocks getPaletteManifest, so its nav assertions now run against the real manifest, and a new case pins /wiki/log and asserts the row is present on the first render with no network at all.
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
1 participant
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.
Summary
Layout.jsxhas importedNAV_COMMANDSstatically since #5053, but it still fetchedGET /api/palette/manifeston every mount to resolve the Pinned/Recent rows whose paths have no sidebar presentation (/wiki/log,/goals/tree, …) and every stored path that needspreviousPathsmigration.Those rows rendered nothing until the request landed and then popped in — and stayed missing for the whole session whenever the request failed (network, or an expired session on a password-gated install), with only a
console.warn.Changes:
manifestEntryByPathis now derived from the manifest the bundle already carries, reusing the module-levelcommandByPathmap thatnavRowForPathalready builds fromNAV_COMMANDS. The memo now depends only onisFeatureEnabled, so the feature gate is unchanged.resolveNavEntrypassesNAV_COMMANDStomigrateLegacyNavPathinstead of the fetched copy.import * as apiinLayout.jsx, whose only consumer was that fetch.The endpoint itself is untouched —
CmdKSearch.jsxandVoiceWidget.jsxstill need itsactionshalf, which only the server can hydrate from the voice tool registry, anddocs/COMPANION_APP_API.mddocuments it for companion apps.Test plan
Layout.test.jsxno longer mocksgetPaletteManifest, so its nav assertions (including the existing eidoversepreviousPathscase) now run against the real manifest rather than a hand-written one-entry fake./wiki/log— a manifest-only path with no sidebar leaf — and asserts the Pinned row is in the DOM on the first render, before anyact()flush. Verified this fails against the pre-fix component even when the mock is widened to return the complete real manifest, so it pins the timing, not just the data.cd client && npm test -- Layout→ 115 passed.server/lib/navManifest.test.js+server/routes/palette.test.jsunchanged → 75 passed.cd client && npm run lintclean.Closes #6362