🎨 Changed and improved the slug generation - #1011
Open
dittnamn wants to merge 6 commits into
Open
Conversation
ref towards TryGhost/Ghost#3224 etc. The old slug generation using unidecode had a lot of issues with erroneous transliteration for many languages, which has been the reason for many discussions and requests for change over the years. By replacing unidecode with anyascii, a lot of these issues should be solved, however not perfectly. Over the years, the initial reasons for not allowing anything other than ascii letters and numbers in the slugs have been fixed. The Ghost databases, routes, links, loading, etc. now support unicode characters in URL:s. Browser support is fully working and many large sites, including Wikipedia, use unicode characters in URL:s. With just a few modifications in the Ghost sources, an option for full unicode slug support could therefore be added for users who want it. To work towards this, an extra option has been added to the slugify function, to allow the disabling of the transliteration part of the slug generation. Another option, allowing the change of slug part separator was also added. Due to how the filtering work, the possible options are currently just spaces, dashes and underscores, but more options could possibly be added in the future. Dots as separators could in theory make good looking slugs, but should be avoided due to the risk of filename mixups. Due to the slightly different transliteration method, some of the tests have been revised and some new ones were added as well. Note that anyascii has some quirks compared to unidecode, but in total this is an improvement for most languages.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughSlugification now uses Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/string/test/slugify.test.js (1)
94-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrevent potential test pollution by scoping
optionslocally.Assigning to
optionswithout declaring it locally modifies a shared variable from the outer suite scope. This can pollute subsequent tests that rely on the defaultoptions, causing them to run with unintended configurations.
packages/string/test/slugify.test.js#L94-L96: Useconst options = {noTransliteration: true};andconst result = ...packages/string/test/slugify.test.js#L101-L103: Useconst options = {separator: ' '};andconst result = ...🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/string/test/slugify.test.js` around lines 94 - 96, Scope the test variables locally in both affected cases: packages/string/test/slugify.test.js lines 94-96 and 101-103. In each test, declare options and result with const inside the test block, replacing assignments to the shared outer-scope variables while preserving the existing option values and slugify calls.packages/string/lib/slugify.js (1)
17-18: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueEnforce allowed values for
options.separator.If
options.separatorcan be controlled by external input, passing special replacement patterns (like$&) could bypass character sanitization, asreplacewould substitute the matched invalid character with itself. Additionally, passing an empty string""would unexpectedly fall back to"-"due to the||operator.Consider validating the separator against the documented allowed values.
🛡️ Proposed fix
- // If the separator isn't set, default to `-` - const separator = options.separator || '-'; + // Ensure the separator is one of the allowed values, default to `-` + const separator = [' ', '_', '-'].includes(options.separator) ? options.separator : '-';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/string/lib/slugify.js` around lines 17 - 18, Update the separator initialization in slugify to validate options.separator against the documented allowed separator values before using it as a replacement string. Preserve an explicitly supported empty-string value instead of treating it as unset, and fall back to "-" only when the value is absent or invalid.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/string/lib/slugify.js`:
- Line 1: Update the any-ascii dependency usage in packages/string so slugify.js
loads successfully under CommonJS: either pin any-ascii to a CommonJS-compatible
version or migrate the package to ESM, ensuring the existing slugify behavior
remains unchanged.
---
Nitpick comments:
In `@packages/string/lib/slugify.js`:
- Around line 17-18: Update the separator initialization in slugify to validate
options.separator against the documented allowed separator values before using
it as a replacement string. Preserve an explicitly supported empty-string value
instead of treating it as unset, and fall back to "-" only when the value is
absent or invalid.
In `@packages/string/test/slugify.test.js`:
- Around line 94-96: Scope the test variables locally in both affected cases:
packages/string/test/slugify.test.js lines 94-96 and 101-103. In each test,
declare options and result with const inside the test block, replacing
assignments to the shared outer-scope variables while preserving the existing
option values and slugify calls.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fd58d1cd-a449-42d6-8881-230376909d5d
📒 Files selected for processing (3)
packages/string/lib/slugify.jspackages/string/package.jsonpackages/string/test/slugify.test.js
The slugify options were renamed to describe their usage better. Also, a NFC normalization has been added to make sure combining marks aren't lost in the conversion and to make sure seemingly identical slugs won't be generated when the unicode slugs are enabled. Extra types of common apostrophes are also now just removed instead of turned into separators, to make sure "what‘s" is turned into "whats" instead of "what-s".
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/string/lib/slugify.js`:
- Around line 12-18: The slugify function’s slugSeparator currently accepts
undocumented values. In the exported slugify function, validate the resolved
separator against the documented choices space, underscore, and hyphen before
replacement processing; throw `@tryghost/errors`’ IncorrectUsageError for
unsupported values, adding the package import if needed.
- Around line 23-31: The Unicode filter in slugify must preserve combining
marks. In packages/string/lib/slugify.js lines 23-31, update the character class
used by slugify to allow Unicode marks alongside letters and numbers; in
packages/string/test/slugify.test.js lines 93-98, add a unicodeSlugs regression
case containing a non-composing combining mark and assert it remains intact in
the generated slug.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a689935d-c40b-4150-910d-e0777d973763
📒 Files selected for processing (2)
packages/string/lib/slugify.jspackages/string/test/slugify.test.js
dittnamn
added a commit
to dittnamn/Ghost-framework
that referenced
this pull request
Jul 30, 2026
ref towards TryGhost/Ghost#3224 etc. In line with the changes in TryGhost/SDK#1011 a change in the validator is necessary to allow unicode slugs to be used. The validator now allows all Unicode letters and numbers, along with spaces, underscores and dashes that can be used as slug separators. Along with the changes in the slugify() function, equivalent changes were made to the security.safe() function to allow it to use the same parameters as slugify() and to make sure the tests will still work with the new transliteration library. However, the security.safe() function is barely used inside Ghost, and could easily be replaced to use slugify() directly. A TODO notice has been added, so that it could potentially be removed in the future.
dittnamn
added a commit
to dittnamn/Ghost
that referenced
this pull request
Jul 30, 2026
fixes TryGhost#3224 The slugs generated by Ghost have had a lot of issues in how they've been transliterated. Through a change in @tryghost/string a new transliteration library, any-ascii, has been proposed to replace unidecode, see TryGhost/SDK#1011 However, to get a fully internationalized site along with SEO optimized URL:s, that change isn't fully enough. The current change adds an option in Labs where it's possible to enable Unicode slugs instead of performing the transliteration. The Unicode characters in use are limited to those that are registered as letters and numbers, which means that emojis, special characters, etc. still will be removed. The only "major" change that has been done in Ghost to make this work is actually just to normalize the URL slugs before looking them up in the database. The rest of the code changes are mostly settings to turn the new feature on or off, and to pass the setting along all the way to the slugify() function. Due to the rest of the system already being in Unicode, everything seems to work as it's supposed to. Instead of using safe() from @tryghost/security, the slug generation has also been changed to utilize slugify() from @tryghost/string directly instead. The routing controllers for rss, collection and channel were previously using safe() as well to slugify the lookups before going to the router, but the entry controller didn't do this. As the router already checks the slugs with isSlug() before passing them further, this was unnecessary and had the potential to break lookups, so it was simply removed. In addition to using Unicode slugs, there's also an option added to switch which separator the slugs use. Previously, dashes (-) were used as the hardcoded default, but for URL readability, underscores (_) could be preferred, like in the URL:s Wikipedia use. There's also an option for using spaces ( ), but this might still be seen as a bit foreign in URL:s. Out of the major browsers, it's currently just Firefox that show these as spaces by default, with Safari doing it in some situations. Other browsers can show the spaces as %20. When activated, the Unicode slugs aren't added to member tags, newsletter, services, integrations or benefits, as these aren't user facing. Note that for this change to work, the changes in TryGhost/SDK#1011 and TryGhost/framework#860 are required.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/string/lib/slugify.js (1)
30-31: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve non-composing Unicode marks end to end.
NFC does not compose every combining mark, while the current filter removes all remaining marks. Update the filter and strengthen the regression test.
packages/string/lib/slugify.js#L30-L31: allow\p{M}alongside letters and numbers.packages/string/test/slugify.test.js#L100-L105: test an input such asa\u0308\u0323and assert the normalized mark remains.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/string/lib/slugify.js` around lines 30 - 31, Update the slugify filter in packages/string/lib/slugify.js lines 30-31 to allow Unicode combining marks via \p{M} alongside letters and numbers. Strengthen the regression test in packages/string/test/slugify.test.js lines 100-105 using input such as a\u0308\u0323 and assert the normalized mark is preserved end to end.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@packages/string/lib/slugify.js`:
- Around line 30-31: Update the slugify filter in packages/string/lib/slugify.js
lines 30-31 to allow Unicode combining marks via \p{M} alongside letters and
numbers. Strengthen the regression test in packages/string/test/slugify.test.js
lines 100-105 using input such as a\u0308\u0323 and assert the normalized mark
is preserved end to end.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 58753286-c00f-4bb5-a56e-ae641a8c3262
📒 Files selected for processing (2)
packages/string/lib/slugify.jspackages/string/test/slugify.test.js
This makes sure Thai script, etc. can be used as slugs, but removes overuse of combining marks that creates Zalgo text and similar.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/string/lib/slugify.js`:
- Around line 30-34: Update the combining-mark cleanup in slugify to also match
runs of four or more marks at the beginning of the slug by allowing a
start-of-string prefix, while preserving the existing behavior for marks
following non-mark characters. Add a regression test covering a leading
combining-mark run when unicodeSlugs is enabled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 73ed724e-72e5-46ee-919c-3bb9cba972d0
📒 Files selected for processing (2)
packages/string/lib/slugify.jspackages/string/test/slugify.test.js
3 tasks
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.
ref towards TryGhost/Ghost#3224 etc.
The old slug generation using unidecode had a lot of issues with erroneous transliteration for many languages, which has been the reason for many discussions and requests for change over the years. By replacing unidecode with anyascii, a lot of these issues should be solved, however not perfectly.
Over the years, the initial reasons for not allowing anything other than ascii letters and numbers in the slugs have been fixed. The Ghost databases, routes, links, loading, etc. now support unicode characters in URL:s. Browser support is fully working and many large sites, including Wikipedia, use unicode characters in URL:s.
With just a few modifications in the Ghost sources, an option for full unicode slug support could therefore be added for users who want it. To work towards this, an extra option has been added to the slugify function, to allow the disabling of the transliteration part of the slug generation.
Another option, allowing the change of slug part separator was also added. Due to how the filtering work, the possible options are currently just spaces, dashes and underscores, but more options could possibly be added in the future. Dots as separators could in theory make good looking slugs, but should be avoided due to the risk of filename mixups.
Due to the slightly different transliteration method, some of the tests have been revised and some new ones were added as well. Note that anyascii has some quirks compared to unidecode, but in total this is an improvement for most languages.
yarn testandyarn lint)