fix: preserve Unicode in routable content slugs - #2505
Conversation
🦋 Changeset detectedLatest commit: 329c503 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 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 |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-playground | 329c503 | Aug 16 2026, 09:09 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-do | 329c503 | Aug 16 2026, 09:10 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-cache | 329c503 | Aug 16 2026, 09:09 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
docs | 3ae8359 | Aug 16 2026, 08:30 PM |
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Unifies slug generation across core/admin to preserve Unicode and introduces a per-collection routable flag (defaulting to true) that requires slugs for publishing and excludes non-routable/slugless content from sitemaps.
Changes:
- Add shared Unicode-preserving
slugifywith grapheme truncation and deterministic fallback for “no usable characters” inputs. - Introduce collection-level
routable(DB migration + schema/API/admin UI + seed import/export) and enforce slug-required publishing whenroutable. - Update SEO/sitemap handlers and expand unit/integration test coverage for routability + Unicode slugs.
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/tests/unit/seed/validate.test.ts | Adds seed validation test for boolean routable. |
| packages/core/tests/unit/seed/apply.test.ts | Verifies applying/updating collection routability via seed. |
| packages/core/tests/unit/schema/registry.test.ts | Tests routable default + updates in schema registry. |
| packages/core/tests/unit/cli/seed-commands.test.ts | Ensures export preserves routable: false. |
| packages/core/tests/unit/api/content-handlers.test.ts | Adds Unicode slug collision + fallback slug + publish validation tests. |
| packages/core/tests/unit/api/collection-admin-schema.test.ts | Extends collection admin schemas to accept/return routable. |
| packages/core/tests/integration/seo/sitemap-route.test.ts | Ensures non-routable collections and slugless entries are not advertised. |
| packages/core/tests/integration/seo/seo.test.ts | Updates sitemap data expectations to exclude slugless published content. |
| packages/core/tests/integration/database/migrations.test.ts | Registers new migration 070_collection_routable. |
| packages/core/src/utils/slugify.ts | Replaces core slugify with re-exported shared implementation + keeps decode. |
| packages/core/src/seed/validate.ts | Validates collection.routable is boolean when present. |
| packages/core/src/seed/types.ts | Adds routable?: boolean to seed collection type. |
| packages/core/src/seed/apply.ts | Persists routable during seed apply/create/update. |
| packages/core/src/schema/types.ts | Adds routable to collection + create/update inputs. |
| packages/core/src/schema/registry.ts | Stores/updates/maps routable to/from DB. |
| packages/core/src/mcp/server.ts | Exposes routable in MCP create/update collection tools. |
| packages/core/src/index.ts | Exports slugify from core public API. |
| packages/core/src/emdash-runtime.ts | Defaults runtime manifest collections to routable. |
| packages/core/src/database/types.ts | Adds routable column to _emdash_collections table type. |
| packages/core/src/database/repositories/content.ts | Enforces slug requirement on publish when requireSlug is true. |
| packages/core/src/database/migrations/runner.ts | Registers migration 070_collection_routable. |
| packages/core/src/database/migrations/070_collection_routable.ts | Adds/drops _emdash_collections.routable with default true. |
| packages/core/src/cli/commands/export-seed.ts | Exports routable: false only (omit when default true). |
| packages/core/src/astro/types.ts | Adds routable?: boolean to manifest collection type. |
| packages/core/src/api/types.ts | Adds routable?: boolean to manifest response collection entries. |
| packages/core/src/api/schemas/schema.ts | Adds routable to create/update bodies and collection schema. |
| packages/core/src/api/handlers/seo.ts | Filters sitemap data to routable collections + excludes slugless rows. |
| packages/core/src/api/handlers/manifest.ts | Includes routable in generated manifest (default true). |
| packages/core/src/api/handlers/content.ts | Passes routable flag into publish path via getCollectionPublishConfig. |
| packages/admin/tsdown.config.ts | Ensures src/slugify.ts is built/typed. |
| packages/admin/tests/lib/utils.test.ts | Expands slugify tests for Unicode preservation, NFKC, fallback, graphemes. |
| packages/admin/tests/components/ContentTypeEditor.test.tsx | Adds routable to fixtures and tests saving routability. |
| packages/admin/src/slugify.ts | Introduces Unicode-preserving slugify implementation w/ fallback + grapheme truncation. |
| packages/admin/src/lib/utils.ts | Re-exports slugify from shared admin implementation. |
| packages/admin/src/lib/api/schema.ts | Adds routable to API client schema types for collections. |
| packages/admin/src/lib/api/client.ts | Adds routable to admin manifest collection type. |
| packages/admin/src/components/ContentTypeEditor.tsx | Adds Routable switch and persists it on save. |
| packages/admin/package.json | Exports ./slugify subpath; adjusts dev script to use config entries. |
| .changeset/unicode-routable-slugs.md | Declares minor bumps and describes routable + Unicode slug behavior. |
Suppressed comments (1)
packages/core/src/api/handlers/seo.ts:1
- Sitemap data excludes
NULLand empty-string slugs, but it will still include whitespace-only slugs (e.g." "), which are treated as empty elsewhere via.trim(). Updating this filter to exclude whitespace-only values (e.g. usingTRIM(c.slug) <> '') would make sitemap behavior consistent with publish validation and avoid advertising invalid legacy slugs.
/**
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
github-actions
Bot
added
the
review/awaiting-author
@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/registry-verification
@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: |
Scope checkThis PR touches 39 files. PRs with a broad scope are harder to review. Please confirm the scope hasn't drifted beyond the intended change. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
|
Addressed the valid review/CI findings in 14c9fdb.
Verified with the focused core regressions (15 passed), the full content-types Playwright spec (11 passed), |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approach judgment: This is the right change. Preserving Unicode in slugs by NFKC-normalising and keeping letters/numbers/marks is a clear improvement over stripping diacritics, and making slug requirements per-collection via a routable flag (defaulting to true) is a backwards-compatible way to support internal/reference content. The implementation is well-scoped and the tests cover the new slug behaviour, the publish-time slug guard, non-routable collections, and sitemap exclusions.
What I checked: the shared @emdash-cms/admin/slugify implementation, the re-export through core, the migration and schema/registry wiring, the publish/seed/scheduled-publish paths, the sitemap query, the admin UI switch, and call-sites of contentRepo.publish().
Headline conclusion: the PR is sound and close to mergeable, but I found two real gaps in the publish-time contract and one seed-resumption correctness issue:
- Direct
status: "published"create/update bypass the routable slug guard.handleContentCreateandhandleContentUpdateaccept astatusfield and write it straight into the content row; neither checks the collection'sroutablesetting or requires a non-empty slug. OnlyhandleContentPublishenforces the rule. API callers can therefore publish a routable entry without a slug without using the publish endpoint. - Seed collection capture fingerprint omits
routable.buildSeedCollectionCaptureFingerprintdoes not includeroutablein the canonicalised payload, so an interrupted seed create that is later resumed after the seed'sroutablevalue changed may recreate the collection with the stale value.
No AGENTS.md convention violations (i18n, RTL, SQL safety, comment discipline, changeset shape) were apparent. The new slug implementation using Intl.Segmenter is appropriate for the supported runtimes, and the core→admin slug re-export is fine because core already depends on @emdash-cms/admin.
Findings
-
[needs fixing]
packages/core/src/api/handlers/content.ts:845handleContentCreatelets a caller publish a routable entry without a slug by passingstatus: "published"with no title. The newroutableguard only exists inhandleContentPublish, so this create path bypasses it entirely.Add the same check before
repo.createwhenbody.status === "published":if (body.status === "published") { const publishConfig = await getCollectionPublishConfig(trx, collection); const intendedSlug = typeof slug === "string" ? slug.trim() : ""; if (publishConfig.routable && !intendedSlug) { throw new EmDashValidationError("Cannot publish routable content without a slug"); } } const created = await repo.create({This mirrors the validation already added to
handleContentPublishand fixes the stated goal that routable entries can no longer be published without a slug. -
[needs fixing]
packages/core/src/api/handlers/content.ts:1045handleContentUpdatealso acceptsstatus: "published"and writes it viarepo.update()without checkingroutableor the slug, so a slugless draft can be promoted to published through an update. The currentexistinglookup is skipped when neither_revnorslugis supplied, so a guard here needs to fetch the row whenever the status changes to published.Introduce the same publish-config check before writing the update:
// Enforce the routable slug rule when an update would publish the entry. if (body.status === "published") { const publishConfig = await getCollectionPublishConfig(trx, collection); if (publishConfig.routable) { const currentSlug = existing?.slug ?? null; const intendedSlug = (typeof body.slug === "string" ? body.slug : currentSlug)?.trim(); if (!intendedSlug) { throw new EmDashValidationError("Cannot publish routable content without a slug"); } } } const updated = await trxRepo.update(collection, resolvedId, { -
[needs fixing]
packages/core/src/schema/registry.ts:199buildSeedCollectionCaptureFingerprintcanonicalises the collection shape used to detect an interrupted seed create/resume, but the newroutablefield is not included. If a seed is interrupted, then the seed file is edited to change onlyroutable, the resumed capture will match the old fingerprint and recreate the collection with the staleroutablevalue.Include
routablein the fingerprint payload:urlPattern: input.urlPattern ?? null, routable: input.routable ?? null, },
|
Addressed all three EmDashBot findings in 48e1fba.
Added failing-then-passing coverage for routable/non-routable direct creates, direct updates with and without a supplied slug, and fingerprint divergence. Focused suites pass 75 tests; package typecheck, |
github-actions
Bot
added
review/needs-rereview
There was a problem hiding this comment.
Approach judgment: this is still the right change. NFKC-normalised Unicode slugs and per-collection routable (default true) solve #2271 cleanly, and the prior review’s two blockers are now fixed.
What changed since the last review:
handleContentCreateandhandleContentUpdatenow enforce the routable slug guard whenstatus === "published", so direct create/update publish can no longer bypass the rule.buildSeedCollectionCaptureFingerprintincludesroutable, so resumed seed creates detect changes to the flag.- Tests cover the direct-status publish bypass for both create and update, and the registry/sitemap tests exercise
routabledefaults and exclusions.
New gaps found:
- The seed path still requires a slug on every content entry and publishes with
requireSlug = truehard-coded, soroutable: falsecollections cannot actually be seeded with slugless published entries. The validator, the apply publish calls, and theSeedContentEntrytype all need to honor the collection setting. - A stale JSDoc describing the old
slugifyimplementation is left attached toformatRelativeTimein@emdash-cms/admin/src/lib/utils.ts.
No Lingui leaks, RTL Tailwind regressions, SQL injection issues, or logged-out query-count increases were introduced. The shared @emdash-cms/admin/slugify export and the core re-export are appropriate for the supported runtimes.
Verdict: comment – the PR is close, but the seed route needs to respect routable before the feature is complete.
Findings
-
[needs fixing]
packages/core/src/seed/apply.ts:561Seed-apply’s update path publishes entries by calling
trxContentRepo.publish(collectionSlug, existing.id)with the defaultrequireSlug=true. That ignores the seeded collection’sroutablevalue and will reject slugless published entries in non-routable collections, even though the API/MCP handlers now passpublishConfig.routable. Thread the collection setting through the positional arguments:await trxContentRepo.setDraftRevision(collectionSlug, existing.id, draft.id); await trxContentRepo.publish( collectionSlug, existing.id, undefined, false, undefined, true, collection.routable !== false, );(Same fix is needed for the create path at line 645.)
-
[needs fixing]
packages/core/src/seed/apply.ts:645The seed create path also hard-codes
requireSlug=truewhen it publishes declaredpublishedentries. Forroutable: falsecollections this will reject slugless entries that the rest of the system now allows. Pass the collection’s routable setting asrequireSlug:if (status === "published") { await trxContentRepo.publish( collectionSlug, item.id, undefined, false, undefined, true, collection.routable !== false, ); } -
[needs fixing]
packages/core/src/seed/validate.ts:621Seed validation rejects any content entry without a slug. A non-routable collection is explicitly allowed to publish slugless entries, so this validator makes that feature unreachable from seeds. Only enforce the slug rule when the entry’s collection is routable:
const collectionRoutable = seed.collections?.find((c) => c.slug === collectionSlug)?.routable ?? true; if (collectionRoutable && !entry.slug) { errors.push(`${prefix}: slug is required`); } -
[needs fixing]
packages/core/src/seed/types.ts:70SeedContentEntry.slugis currently required, but non-routable collections are allowed to publish slugless entries. The type should allow omitting (or nulling) the slug so hand-written seeds can exercise that path:/** URL slug (omit for slugless entries in non-routable collections) */ slug?: string | null; -
[needs fixing]
packages/admin/src/lib/utils.ts:31-35This JSDoc described the old inline
slugifyimplementation, which was removed and re-exported from../slugify.js. It is now factually wrong and attached toformatRelativeTime. Per AGENTS.md comment discipline, delete stale/justifying comments.} return new Date(value); } export function formatRelativeTime(dateString: string): string {
github-actions
Bot
added
review/needs-rereview
There was a problem hiding this comment.
This is the right shape for #2271: a shared Unicode-aware slug implementation plus a per-collection routable flag cleanly separate public URL content from internal/reference content, and the data-integrity holes from the first review are closed.
On this re-review I verified that the prior round's items are resolved: the stale issue-reference comment is gone, handleContentSchedule now validates routability before persisting the schedule, export-seed.ts correctly preserves "0" slugs and omits slug for non-routable collections, the migration/schema/registry/fingerprint/manifest/MCP wiring all carry routable, and the admin Routable switch/helper-text strings are Lingui-wrapped.
What I newly checked in detail: the shared slugify implementation, the create/update/publish/schedule paths for falsy-slug handling, the sitemap query predicate, seed apply/export/validate round-trips, and the admin collection-editor form wiring. I did not run the test suite or build.
Headline: no blockers; two line-level findings remain — one logic bug where a valid "0" slug is silently discarded on creation (and a related redirect edge on update), and one comment-discipline note in the admin editor JSX.
Findings
-
[needs fixing]
packages/core/src/api/handlers/content.ts:839-840The create path uses
if (!slug)to decide whether to auto-generate a slug from the title. That check incorrectly treats the valid slug"0"as missing (!"0"istrue), so a caller who explicitly passesslug: "0"will silently get a different slug generated from the title. The content schema and export-seed both treat"0"as a legitimate slug, so discarding it here is data loss.Use an explicit "absent or empty" check instead of truthiness so that
"0"is preserved whileundefined,null, and""still trigger auto-generation.let slug: string | null | undefined = body.slug; if (slug === undefined || slug === null || slug === "") { const slugSource = getSlugSource(body.data); if (slugSource) { slug = await repo.generateUniqueSlug(collection, slugSource, effectiveLocale); } } -
[needs fixing]
packages/core/src/api/handlers/content.ts:1050The redirect-capture check has the same truthiness trap:
body.slug && …is false whenbody.slugis"0", so changing a slug to or from"0"skips the 301 redirect that the update path is supposed to leave behind. (The same edge case applies to any other slug that happens to be falsy in JavaScript.)Check that
body.slugis a non-empty string rather than relying on truthiness.if ( typeof body.slug === "string" && body.slug.trim().length > 0 && existing?.slug && existing.slug !== body.slug ) { -
[suggestion]
packages/admin/src/components/ContentTypeEditor.tsx:333-335This JSX comment justifies the design choice of keeping both a sticky-header save button and a bottom-of-form save button. Per AGENTS.md comment discipline, comments should not justify decisions or narrate rejected alternatives; the code (two save buttons, one inside the form for DOM order) should be self-explanatory, or the rationale belongs in a commit message or design doc.
<EditorHeader leading={
github-actions
Bot
added
review/awaiting-author
|
Addressed the latest review on current head 041ca5c.
Verified after the change with formatting, lint:quick, the admin package typecheck, and type-aware lint:json (0 diagnostics). No inline threads were created for these three summary findings, so this comment records the evidence and resolution. |
github-actions
Bot
added
review/needs-rereview
There was a problem hiding this comment.
This is still the right shape for #2271: a single Unicode-aware slug implementation plus a per-collection routable option cleanly separates public URL content from internal/reference content, and the migration/schema/seed/admin wiring looks complete. I did not run the test suite or build.
On this re-review I confirmed that the prior comment-discipline finding is resolved (the sticky-header justification comment in ContentTypeEditor.tsx is gone). However, two truthiness-related regressions flagged in the previous round are still present in packages/core/src/api/handlers/content.ts, and fixing the redirect capture reveals a third adjacent guard with the same problem. The create path and the update redirect logic both treat the valid slug "0" as absent, which contradicts the rest of the PR where "0" is explicitly preserved as a legitimate slug.
Headline: three small but real edge-case bugs remain; once they are fixed, this should be good to merge.
Findings
-
[needs fixing]
packages/core/src/api/handlers/content.ts:840The create path still uses
if (!slug)to decide whether to auto-generate a slug from the title. This was flagged in the previous review as a regression for valid"0"slugs:!"0"istrue, so a caller who explicitly passesslug: "0"silently gets a different slug generated from the title instead. The schema, export-seed, andrequireRoutablePublishSlugall treat"0"as a legitimate slug, so this is data loss.Use an explicit "absent or empty" check so that
"0"is preserved whileundefined,null, and""still trigger auto-generation.let slug: string | null | undefined = body.slug; if (slug === undefined || slug === null || slug === "") { const slugSource = getSlugSource(body.data); if (slugSource) { slug = await repo.generateUniqueSlug(collection, slugSource, effectiveLocale); } } -
[needs fixing]
packages/core/src/api/handlers/content.ts:1050The redirect-capture check still relies on truthiness:
body.slug && ...is false whenbody.slugis the valid string"0". That means updating a slug to"0"skips the old-slug capture, so no 301 redirect is recorded. (Changing from"0"also breaks because the new slug is skipped.) This is the same falsy-slug bug reported in the previous round.Check that
body.slugis a non-empty string rather than relying on truthiness.if ( typeof body.slug === "string" && body.slug.trim().length > 0 && existing?.slug && existing.slug !== body.slug ) { -
[needs fixing]
packages/core/src/api/handlers/content.ts:1084Even if the old-slug capture above is fixed, the redirect-creation guard still uses
body.slugas a truthiness test. When the new slug is"0",oldSlug && body.slugevaluates to"0"(falsy), socreateSlugChangeRedirectis never called. Use a non-empty-string check here, too.if (oldSlug && typeof body.slug === "string" && body.slug.trim().length > 0) { await createSlugChangeRedirect(trx, collection, oldSlug, body.slug, resolvedId); }
github-actions
Bot
added
review/awaiting-author
|
The latest re-review repeats the same incorrect premise, so no production change is warranted. Node 22 executable result on the exact values: {"notZero":false,"booleanZero":true,"redirectGuard":true}Concretely:
Only numeric 0 is falsy; the slug is a string. The valid ContentTypeEditor comment finding is already fixed in 041ca5c. Formatting, admin typecheck, lint:quick, and type-aware lint:json (0 diagnostics) pass. No inline threads were created for this review, so there is nothing to resolve in the thread UI. |
|
Fixed the branch-caused Tests failure in 329c503. The failing integration case created a default-routable plain_post row with status "published" but no slug through the low-level repository, then republished it. That fixture violated the new routable publish invariant and failed identically on SQLite and Postgres. The fixture now supplies slug: "plain-post"; the production guard remains unchanged. Reproduced locally before the fix, then verified the focused integration file passes (5/5), along with formatting, lint:quick, the emdash package typecheck, and type-aware lint:json (0 diagnostics). |
github-actions
Bot
added
review/needs-rereview
There was a problem hiding this comment.
This is still the right shape for #2271: a single Unicode-aware slugify implementation shared between core and admin, plus a per-collection routable option, cleanly separates public URL content from internal/reference content. The migration, schema, seed, sitemap, and admin wiring look complete.
On this re-review the prior comment-discipline finding remains resolved, but the three truthiness-related regressions I flagged previously are still present in packages/core/src/api/handlers/content.ts. In addition, tracing the same "falsy slug" pattern through the changed code surfaced the same bug in two more places: the publish-time redirect guard and the auto-slug generator in the content repository. Because the PR explicitly treats "0" as a valid slug, every !slug, slug &&, and if (slug) guard that conflates "0" with absence is now a regression.
Headline: five small but real edge-case bugs remain; once they are fixed and covered by regression tests, this should be good to merge.
Findings
-
[needs fixing]
packages/core/src/api/handlers/content.ts:840The create path still uses
if (!slug)to decide whether to auto-generate a slug from the title.!"0"istrue, so a caller who explicitly passesslug: "0"silently gets a different slug generated from the title instead. The schema, seed validation, andrequireRoutablePublishSlugall treat"0"as a legitimate slug, so this is data loss.Use an explicit "absent or empty" check so that
"0"is preserved whileundefined,null, and""still trigger auto-generation.let slug: string | null | undefined = body.slug; if (slug === undefined || slug === null || slug === "") { const slugSource = getSlugSource(body.data); if (slugSource) { slug = await repo.generateUniqueSlug(collection, slugSource, effectiveLocale); } } -
[needs fixing]
packages/core/src/api/handlers/content.ts:1050The redirect-capture check still relies on truthiness:
body.slug && ...is false whenbody.slugis the valid string"0", so updating a slug to"0"skips the old-slug capture. The same guard also relies onexisting?.slugbeing truthy, so changing from"0"is also missed. Both cases break 301 redirect recording for a slug the PR explicitly supports.Check that both slugs are non-empty strings rather than relying on truthiness.
if ( typeof body.slug === "string" && body.slug.trim().length > 0 && typeof existing?.slug === "string" && existing.slug.trim().length > 0 && existing.slug !== body.slug ) { oldSlug = existing.slug; } -
[needs fixing]
packages/core/src/api/handlers/content.ts:1084Even if the old-slug capture above is fixed, the redirect-creation guard still uses
body.slugas a truthiness test. When the new slug is"0",oldSlug && body.slugevaluates to"0"(falsy), socreateSlugChangeRedirectis never called.Use a non-empty-string check for both the old and new slug.
if ( typeof oldSlug === "string" && oldSlug.trim().length > 0 && typeof body.slug === "string" && body.slug.trim().length > 0 ) { await createSlugChangeRedirect(trx, collection, oldSlug, body.slug, resolvedId); } -
[needs fixing]
packages/core/src/api/handlers/content.ts:1615-1618The publish-time redirect guard also relies on slug truthiness:
existing.slug && published.slug && .... If either the old slug or the newly-published slug is"0", the guard short-circuits and no 301 redirect is created for a slug change that went live during publish. This is the same falsy-slug bug in the revision/publish path the PR introduced.Check both slugs explicitly.
if ( existing?.status === "published" && typeof existing.slug === "string" && existing.slug.trim().length > 0 && typeof published.slug === "string" && published.slug.trim().length > 0 && existing.slug !== published.slug ) { await createSlugChangeRedirect(trx, collection, existing.slug, published.slug, resolvedId); } -
[needs fixing]
packages/core/src/database/repositories/content.ts:414generateUniqueSlugtreats a slug of"0"as empty:!"0"istrue, so a title that normalizes to"0"(e.g. the literal title"0"or an Arabic-Indic digit that NFKC-normalizes to"0") causes the function to returnnullinstead of generating the slug"0". For a routable collection this leaves the entry without a slug and blocks publishing; for a draft it silently drops a valid slug.Replace the truthiness check with a length check.
const baseSlug = slugify(text); if (baseSlug.trim().length === 0) return null;
github-actions
Bot
added
review/awaiting-author
|
The current-head review repeats the same incorrect JavaScript premise across five sites. Exact Node 22 evaluation of those guards: {"notZero":false,"createAutoGenerates":false,"captureToZero":true,"captureFromZero":true,"createRedirect":true,"publishRedirect":true,"repositoryReturnsNull":false}The slug type is string. Numeric 0 is falsy; string "0" is truthy. Therefore:
These findings are not actionable, and changing correct guards would only restate JavaScript truthiness without changing behavior. The valid comment finding was fixed, the branch-caused integration fixture failure was fixed in 329c503, all review threads are resolved, and both current-head CI matrices are green. No inline threads were created for this review. |
What does this PR do?
Adds one browser-safe Unicode slug implementation shared by core and admin. It NFKC-normalizes and lowercases text, preserves Unicode letters, numbers, and combining marks, truncates by grapheme, and uses a deterministic fallback when no usable characters remain. Database-backed collision suffixing remains in the content repository.
Adds a per-collection
routableoption, defaulting totrue. Routable entries can no longer be published with a null or empty slug, including a staged empty slug replacing a live one. Non-routable collections remain available for internal/reference-only content, and slugless/non-routable entries are not advertised in sitemaps.Closes #2271
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
pnpm --filter @emdash-cms/admin --filter emdash buildpnpm typecheckpnpm lintpnpm lint:json | jq '.diagnostics | length'→0dir="rtl",lang="ar"; the Routable switch rendered without clipping and toggled correctly.Try this PR
Open a fresh playground →
A full working EmDash site, deployed from this branch. Each visit gets its own session-scoped sandbox: no login needed and no shared state. Try the admin, edit content, hit the public site.
Tracks
codex/unicode-slugs-2271. Updated automatically when the playground redeploys.