[codex] Add LiveSearch route templates#1387
Conversation
🦋 Changeset detectedLatest commit: 5ae4889 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 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 |
|
/review |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
There was a problem hiding this comment.
This PR fixes a real gap: LiveSearch previously hardcoded /{collection}/{slug} for all result links, which breaks sites whose public routes differ. The approach—extracting URL generation into buildLiveSearchResultUrl, adding routeMap/searchPage/style props, and returning slug from the suggestion query so both modes can use slug-based templates—is clean and matches existing full-search behavior.
I found two implementation bugs and one inconsistency:
replaceAllreplacement-string interpolation inbuildLiveSearchResultUrl:String.prototype.replaceAlltreats$sequences ($$,$&,$',$`,$n) as special patterns in the replacement string. If a resultid,slug, orcollectioncontains$, the generated URL is corrupted.- Missing
.jsextension on the internal ESM import inside the<script>tag (AGENTS.md convention: "Internal imports use.jsextensions"). - Suggestion links omit
data-slugeven though suggestions now carryslug, creating an inconsistency withrenderResults.
The SQL addition (c.slug in getSuggestions) is safe and consistent with the existing full-search query. The changeset and targeted tests are present. No locale-filtering or authorization regressions.
|
/review |
|
This PR has been inactive for 14 days. It will be closed automatically in 7 days if there is no further activity. If you're still working on this, please push an update or leave a comment. |
There was a problem hiding this comment.
This re-review confirms the PR is now clean.
The approach is still sound: extracting URL generation into buildLiveSearchResultUrl, adding routeMap/searchPage/style props, and returning slug from the suggestions query so both full-search and suggest mode can use slug-based templates correctly fits EmDash's architecture.
All three findings from the previous review are addressed:
replaceAllreplacement-string interpolation — fixed by usingtemplate.split(token).join(value)inbuildLiveSearchResultUrl, which treats$sequences as literal text.- Missing
.jsextension on the internal ESM import — fixed; the<script>block now imports from"./live-search-routing.js". - Suggestion links omitting
data-slug— fixed;renderSuggestionsnow setslink.dataset.slug = suggestion.slug ?? "", consistent withrenderResults.
I re-read the changed files and searched the package exports / sibling components. No new blocking issues, security gaps, or AGENTS.md convention violations were introduced. The targeted unit test for buildLiveSearchResultUrl now covers dollar-sign edge cases, the integration test verifies slug is returned from getSuggestions, and the changeset is present.
Verdict: approve.
What does this PR do?
Fixes LiveSearch result navigation for sites whose public routes do not match the default
/{collection}/{slug}shape.This PR:
routeMapprop so sites can map collection results to public URL templates like/item/:slug.searchPageprop so pressing Enter without a focused result can navigate to a full search page with?q=set.stylepassthrough for the LiveSearch container, matching the existing class passthrough.slugfrom autocomplete suggestions sosuggestModecan use slug-based route templates too.Closes N/A
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain.AI-generated code disclosure
Screenshots / test output
Validation run locally:
pnpm --silent lint:json | jq '.diagnostics | length'->0pnpm lintpnpm typecheckpnpm --dir packages/core exec vitest run tests/unit/components/live-search-routing.test.ts tests/integration/search/suggest.test.tspnpm --dir packages/core typecheckpnpm --filter emdash buildpnpm formatgit diff --check