refactor(utils): add slugify and adopt it at the eight sites that hand-rolled it - #7018
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryLow Risk Overview Callers still own truncation and empty-string fallbacks (org slugs, chat identifiers, skill names). Behavior is intended to stay identical; tests cover empty, punctuation-only, non-Latin, and already-hyphenated inputs. Reviewed by Cursor Bugbot for commit 50594b8. Configure here. |
Greptile SummaryThis PR centralizes eight equivalent slug derivations in a new
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/utils/src/string.ts | Adds a documented shared slug helper whose transformation matches the migrated implementations. |
| packages/utils/src/string.test.ts | Covers lowercasing, separator collapsing, boundary stripping, empty inputs, non-ASCII input, digits, and existing hyphens. |
| apps/sim/app/api/v1/admin/organizations/route.ts | Replaces the local organization-slug derivation while preserving requested-slug precedence. |
| apps/sim/ee/workspace-forking/lib/copy/copy-chats.ts | Adopts the shared derivation while retaining the 24-character limit, trailing-hyphen cleanup, and fallback. |
| apps/sim/lib/billing/enterprise-owner-claim.ts | Adopts the shared helper while preserving truncation, fallback, and claim-specific suffix behavior. |
| apps/sim/lib/billing/enterprise-provisioning.ts | Adopts the shared helper while preserving truncation, fallback, and organization-ID suffix behavior. |
| apps/sim/lib/organizations/instance-org.ts | Reuses the canonical derivation while retaining explicit handling for unusable empty slugs. |
| apps/sim/scripts/consolidate-users-into-organization.ts | Uses the same shared derivation consistently for organization lookup and creation. |
Reviews (2): Last reviewed commit: "refactor(utils): add slugify and adopt i..." | Re-trigger Greptile
53b4dfd to
af05283
Compare
…d-rolled it The same three-step derivation — lowercase, collapse each non-alphanumeric run to a hyphen, strip the leading and trailing one — sat in eight files. Two of them carried a TSDoc line whose only job was to warn that they mirrored a third (`instance-org.ts`: "Derives a slug the same way the admin organization API does"; `consolidate-users-into-organization.ts`: "Mirrors the slug derivation used by POST /api/v1/admin/organizations"). A comment asserting two implementations agree is the shape duplication takes when it cannot be checked. All eight were semantically identical. Two anchored the strip with `-+` rather than `-`, and one followed it with a `--+` collapse, but `[^a-z0-9]+` has already collapsed every run by that point, so neither could ever match more than the single-hyphen form. Nothing changes. Truncation stays at the call sites. Four of them bound the result — at 24, 64 and 80 — and only `copy-chats.ts` strips again afterwards, because slicing can land mid-run and leave a trailing hyphen the earlier strip never saw. Folding a `maxLength` into the helper would have had to pick one of those behaviors and silently impose it on the others. `artifact-stylesheet.ts` keeps its copy: it lives inside the `SIM_ARTIFACT_SHELL` template literal and runs in the viewer's browser, where there is no import to resolve.
50594b8 to
fdc4038
Compare
What
The same three-step derivation — lowercase, collapse each non-alphanumeric run to a hyphen, strip the leading and trailing one — sat in eight files.
Two of them carried a comment whose only job was to warn that they mirrored a third:
instance-org.ts: "Derives a slug the same way the admin organization API does."consolidate-users-into-organization.ts: "Mirrors the slug derivation used byPOST /api/v1/admin/organizations."A comment asserting that two implementations agree is what duplication looks like when nothing can check it.
Verified identical, not merely similar
All eight produce the same output. Two anchored the strip with
-+rather than-, and one followed it with a--+collapse — but[^a-z0-9]+has already collapsed every run by that point, so at most one leading and one trailing hyphen can exist and neither variant can ever match more than the single-hyphen form..trim()in the skills helper is likewise a no-op: leading whitespace becomes a hyphen that is then stripped.What stayed at the call sites
Truncation. Four sites bound the result — at 24, 64 and 80 — and only
copy-chats.tsstrips again afterwards, because slicing can land mid-run and leave a trailing hyphen the earlier strip never saw. AmaxLengthoption would have had to pick one of those behaviors and impose it on the others, so the helper does the derivation and callers keep the bounding.Fallbacks. Three sites need a non-empty result (
'organization','chat','section'). What to fall back to is domain knowledge, so it stays with the caller.One site deliberately not migrated
artifact-stylesheet.ts:687keeps its copy. It lives inside theSIM_ARTIFACT_SHELLtemplate literal and runs in the viewer's browser, where there is no import to resolve — migrating it would emitslugify is not definedinto every generated artifact page.Also considered and rejected
getFileExtensionis byte-identical to a privateextractExtensionin the same directory. Deduping those three lines would cost a 32-file import churn, an import cycle, or a re-export from a non-barrel file (which CLAUDE.md bans). Left alone.Testing
slugifyhas direct tests covering the empty, all-punctuation, non-Latin, and already-hyphenated cases.packages/utils; 1361 across the touchedapps/simareas.bun run type-checkclean.