New string package: add kebab-case utility and migrate private API calls - #81294
New string package: add kebab-case utility and migrate private API calls#81294oandregal wants to merge 11 commits into
string package: add kebab-case utility and migrate private API calls#81294Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: -205 B (0%) Total Size: 7.82 MB 📦 View Changed
|
There was a problem hiding this comment.
Pull request overview
Introduces @wordpress/kebab-case as a public utility and migrates consumers away from the Components private API.
Changes:
- Adds the package implementation, tests, documentation, and metadata.
- Migrates six packages to direct imports.
- Updates dependencies, TypeScript references, changelogs, and documentation.
Reviewed changes
Copilot reviewed 40 out of 42 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tsconfig.json |
References the new package. |
packages/kebab-case/tsconfig.json |
Adds TypeScript configuration. |
packages/kebab-case/src/test/index.js |
Houses parity tests. |
packages/kebab-case/src/index.ts |
Exposes kebabCase. |
packages/kebab-case/README.md |
Documents usage and semantics. |
packages/kebab-case/package.json |
Defines package metadata. |
packages/kebab-case/CHANGELOG.md |
Records the initial release. |
packages/global-styles-ui/tsconfig.json |
Adds the package reference. |
packages/global-styles-ui/src/font-library/utils/make-families-from-faces.ts |
Uses the public utility. |
packages/global-styles-ui/src/font-library/utils/index.ts |
Uses the public utility. |
packages/global-styles-ui/package.json |
Adds the dependency. |
packages/global-styles-ui/CHANGELOG.md |
Records the migration. |
packages/editor/tsconfig.json |
Adds the package reference. |
packages/editor/src/components/post-actions/index.js |
Migrates modal slug generation. |
packages/editor/package.json |
Adds the dependency. |
packages/editor/CHANGELOG.md |
Records the migration. |
packages/dataviews/tsconfig.json |
Adds the package reference. |
packages/dataviews/src/components/dataviews-item-actions/index.tsx |
Replaces the local utility. |
packages/dataviews/package.json |
Adds the dependency. |
packages/dataviews/CHANGELOG.md |
Records the migration. |
packages/components/tsconfig.json |
Adds the package reference. |
packages/components/src/utils/test/strings.js |
Removes relocated tests. |
packages/components/src/utils/strings.ts |
Removes the private implementation. |
packages/components/src/private-apis.ts |
Removes the private export. |
packages/components/src/palette-edit/index.tsx |
Uses the public utility. |
packages/components/package.json |
Adds the dependency. |
packages/components/CHANGELOG.md |
Records the extraction. |
packages/block-library/tsconfig.json |
Adds the package reference. |
packages/block-library/src/embed/util.js |
Migrates provider slug generation. |
packages/block-library/package.json |
Adds the dependency. |
packages/block-library/CHANGELOG.md |
Records the migration. |
packages/block-editor/tsconfig.json |
Adds the package reference. |
packages/block-editor/src/hooks/use-typography-props.js |
Migrates typography slugs. |
packages/block-editor/src/hooks/layout.js |
Migrates layout slugs. |
packages/block-editor/src/hooks/font-family.js |
Migrates font-family slugs. |
packages/block-editor/src/components/font-sizes/utils.js |
Migrates font-size slugs. |
packages/block-editor/src/components/colors/with-colors.js |
Migrates color property slugs. |
packages/block-editor/src/components/colors/utils.js |
Migrates color class slugs. |
packages/block-editor/package.json |
Adds the dependency. |
packages/block-editor/CHANGELOG.md |
Records the migration. |
package-lock.json |
Locks the workspace and dependencies. |
docs/manifest.json |
Registers the package documentation. |
|
I wonder if this should be a "string" package like we have a "url" package... to potentially absorb similar string utils later. |
Personally I'd hope we would be able to use off-the-shelf libraries like we're using |
I don't believe that's a big risk though, we can make this a bundled package with a clear purpose (string manipulation functions that we maintain ourselves) and since it's also a bundled package, breaking changes and all are ok. |
Thanks for the ping. What memories! Short answer: no nothing was intentional in the style engine aside from focussing on the theme json preset parsing. It probably predates a few use cases we need today. I see no downside in swapping out for this PR's implementation. I ran some tests locally to check. During that test, a couple of ASCII related things came up on the parity to `kebabCase` parity data ASCII(ASCII)Two families, both narrow, both fixable in the file this PR is moving. (a) The apostrophe strip is missing the (b) An ordinal immediately followed by a digit. PHP's
Non-ASCIIPHP's
This is the one place where swapping As for the code in Hope that helps! |
I agree with a string package. Sooner or later we will end-up needing to maintain other string manipulation function like this one where using a third-party lib is not possible because of a specific requirement, having a package with documentation saying that when possible existing third-party packages should be used. And on each function explain why a third-party was not used seems like it may save the effort of creating another package in the future. |
Extracts the WP-parity kebabCase utility (backwards compatible with Lodash's _.kebabCase() and Core's _wp_to_kebab_case()) into a new dependency-only package (no wpScript), and migrates the two identical copies: - @wordpress/components: utils/strings.ts now re-exports kebabCase from the new package, so the existing private API surface and all unlock() consumers are unchanged. The kebabCase test suite moves to the new package. - @wordpress/dataviews: removes the copy inlined in #81284 and imports the package directly. Follow-up to #81284 and part of the direction discussed in #81230; the utility originally became a components private API in #56758. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Removes kebabCase from the @wordpress/components private APIs and updates every consumer to import the new package directly: - block-editor: colors (utils, with-colors), font-sizes utils, and the font-family, layout, and use-typography-props hooks - block-library: embed util - editor: post-actions (Menu remains unlocked) - global-styles-ui: font-library utils - components itself: palette-edit, and utils/strings.ts no longer re-exports it Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The package hosts string utilities that WordPress maintains itself, starting with kebabCase. See the discussion in #81294.
Trunk's ESLint config no longer allows dependency group comments.
@wordpress/string is a public npm package without a WordPress script build. Without this exclusion, dependency-extraction-webpack-plugin externalizes it to the nonexistent wp-string handle and wp.string global, so plugin scripts importing it fail to enqueue.
8883f00 to
8b4065c
Compare
github-actions
Bot
added
[Package] Components
The rename from @wordpress/kebab-case left the dependency at kebab-case's sort position in six consumer package.json files.
| @@ -1,99 +1,4 @@ | |||
| import { kebabCase, normalizeTextString } from '../strings'; | |||
There was a problem hiding this comment.
These tests have been migrated verbatim to packages/string/src/test/index.js.
| export function kebabCase( str: unknown ) { | ||
| let input = str?.toString?.() ?? ''; | ||
|
|
||
| // See https://github.com/lodash/lodash/blob/b185fcee26b2133bd071f4aaca14b455c2ed1008/lodash.js#L4970 | ||
| input = input.replace( /['\u2019]/, '' ); | ||
|
|
||
| return paramCase( input, { | ||
| splitRegexp: [ | ||
| /(?!(?:1ST|2ND|3RD|[4-9]TH)(?![a-z]))([a-z0-9])([A-Z])/g, // fooBar => foo-bar, 3Bar => 3-bar | ||
| /(?!(?:1st|2nd|3rd|[4-9]th)(?![a-z]))([0-9])([a-z])/g, // 3bar => 3-bar | ||
| /([A-Za-z])([0-9])/g, // Foo3 => foo-3, foo3 => foo-3 | ||
| /([A-Z])([A-Z][a-z])/g, // FOOBar => foo-bar | ||
| ], | ||
| } ); | ||
| } |
There was a problem hiding this comment.
This has been migrated verbatim to packages/string/src/index.js.
oandregal
changed the title
kebab-case package and migrate private API callsstring package: add kebab-case utility and migrate private API calls
|
Flaky tests detected in f375665. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/31177108322 As a user I want to be able to add multiple close buttons so that users can close the overlay from different positions in
|
State that kebabCase is designed to match _wp_to_kebab_case() with the test suite covering every Core test case, rather than claiming blanket parity, and use only examples that appear in the test suite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Declare an explicit "wpScript": false, use "default" instead of "require" in the exports map, and bump the engines requirement, matching the conventions of the most recently added bundled packages (grid, ui, style-runtime). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The reference kept the slot that was alphabetical for the former kebab-case package name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@ramonjd thanks for that additional context and tests. I want this PR to be focused on extracting the existing utility verbatim. But those additional tests you brought up (also what copilot found) merit looking into them separately. If the client and server implementation are not aligned, they should. Though I suppose it's not a practical issue, otherwise we'd have heard users feedback? Anyway, I don't feel comfortable migrating |
|
re: I think this is now ready. |
| @@ -1,5 +1,4 @@ | |||
| import removeAccents from 'remove-accents'; | |||
| import { paramCase } from 'change-case'; | |||
|
|
|||
There was a problem hiding this comment.
What about the other functions here in this file, are they good candidates for the string package?
youknowriad
left a comment
youknowriad
left a comment
There was a problem hiding this comment.
I prefer string personally but also not strongly opinionated either way. Feel free to move forward either way.
|
I share a similar sentiment to @aduth (which reminds me of the many times that I've got a "YAGNI" from @tyxla ) — should we actually create a string package only if/when we really need it? My instinct tells me that 99% of the functionality that we need can be provided by a trusted 3rd party library, such as https://www.npmjs.com/package/es-toolkit or https://www.npmjs.com/package/@stdlib/string (or via a mix of smaller, more specialized packages, starting from https://www.npmjs.com/package/change-case for this specific scenario?) |
tyxla
left a comment
tyxla
left a comment
There was a problem hiding this comment.
I don't have strong feelings either way, but as @ciampo pointed out, I always suggest the YAGNI mentality.
To be fair, what turns me away from the string package idea is that we're risking that it becomes a string function kitchen sink, a mini version of Lodash, which I'd personally recommend against.
| "engines": { | ||
| "node": ">=18.12.0", | ||
| "npm": ">=8.19.2" | ||
| } |
There was a problem hiding this comment.
I believe we need to regenerate package-lock so this is consistent with the versions the package is actually declaring
|
|
||
| Converts any string to kebab-case. | ||
|
|
||
| The conversion is designed to match WordPress Core's [`_wp_to_kebab_case()`](https://developer.wordpress.org/reference/functions/_wp_to_kebab_case/) function (and, transitively, Lodash's [`_.kebabCase()`](https://lodash.com/docs/4.17.15#kebabCase)): the package's test suite covers every test case of the Core function. These semantics differ from other kebab-case implementations in how numbers are handled (`'white23'` becomes `'white-23'`, `'font2xl'` becomes `'font-2-xl'`, yet `'white4th'` stays `'white-4th'`). WordPress uses these semantics when generating slugs for CSS class names and preset CSS custom properties (`--wp--preset--*`), so use this function whenever a generated slug needs to match one generated by WordPress. |
There was a problem hiding this comment.
One thing to keep in mind here: we're conflating kebab case with slug generation, almost as if they're the same thing. Don't we allow the possibility that we want to kebab case something that is not used for a slug?
|
|
||
| Converts any string to kebab-case. | ||
|
|
||
| The conversion is designed to match WordPress Core's [`_wp_to_kebab_case()`](https://developer.wordpress.org/reference/functions/_wp_to_kebab_case/) function (and, transitively, Lodash's [`_.kebabCase()`](https://lodash.com/docs/4.17.15#kebabCase)): the package's test suite covers every test case of the Core function. These semantics differ from other kebab-case implementations in how numbers are handled (`'white23'` becomes `'white-23'`, `'font2xl'` becomes `'font-2-xl'`, yet `'white4th'` stays `'white-4th'`). WordPress uses these semantics when generating slugs for CSS class names and preset CSS custom properties (`--wp--preset--*`), so use this function whenever a generated slug needs to match one generated by WordPress. |
There was a problem hiding this comment.
Should we clarify any exceptions to "matching WordPress core's _wp_to_kebab_case()? For example how we handle any accented characters, or how we handle a''a (a-a here vs aa in Core)
| "@wordpress/string": "file:../string", | ||
| "@wordpress/ui": "file:../ui", | ||
| "@wordpress/warning": "file:../warning", | ||
| "change-case": "^4.1.2", |
There was a problem hiding this comment.
Looks like change-case may no longer be a direct dependency after this change.
Follow-up to #81284; see #81230 and #56758.
What?
Introduces a new
@wordpress/kebab-casepackage, and migrates the existing usage of the private utility.Why?
We need a
kebabCaseutility that is backwards compatible with Lodash's_.kebabCase()and Core's_wp_to_kebab_case(). This utility was deemed private as of #56758 However, the API is stable, tested and won't change because we've locked its implementation to behave like its server-side equivalent (_wp_to_kebab_case). By making it public, the contract is made explicit.Additionally, working with private APIs has proven problematic (see #81230, for example), and we should minimize/remove them as much as possible.
How?
packages/kebab-case: carries the implementation verbatim, plus the full test suite including all_wp_to_kebab_case()parity cases.Questions
There are two semi-identical implementations that diverge from the canonical semantics:
packages/global-styles-engine/src/utils/string.ts.packages/style-engine/src/styles/utils.ts._wp_to_kebab_caseglobal-styles-enginestyle-enginefooBarfoo-barfoo-barfoo-barwhite2whitewhite-2-whitewhite-2-whitewhite-2-whitewhite2ndwhite-2ndwhite-2-ndwhite-2-ndwhite2ndColorwhite-2nd-colorwhite-2-nd-colorwhite-2-nd-colorFOOBarfoo-barfoobarfoo-barWHITEtoWHITEwhit-eto-whitewhiteto-whitewhit-eto-whitei've doneive-donei've-donei-ve-done#ffffffffffff#ffffffffffffwhite+blackwhite-blackwhite+blackwhite-blackfoo barfoo-bar-foo-bar-foo-barcc @ramonjd for thoughts: are these implementations intentionally divergent from
_wp_to_kebab_case? I haven't dug into them or how they are used, and so I may be missing tons of context.Testing Instructions
kebabCaseremains:git grep -n "kebabCase" -- packages | grep unlockshould return nothing.is-provider-youtubeclass.has-{slug}-colorclass.dataviews-action-modal__duplicate-patternclass.Use of AI Tools
This PR was written with Claude Code (implementation, tests, and this description), under human direction and review.