Components: Move kebabCase() function from block-editor package and mark it as private API #56758
Conversation
This file contains 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
…ark it as private API
|
Size Change: -152 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 102e985. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7189905886
|
t-hamano
commented
Dec 4, 2023
t-hamano
added
[Package] Components
|
As mentioned in this comment, I am trying to move the However, the Mobile Unit Test is failing and I haven't been able to resolve it yet. If you know anything about this please let me know 🙏 |
Yes, this is because we have two separate versions of the components package - one for web and one for React Native. Then the web one exports the private APIs: gutenberg/packages/components/src/index.ts Line 222 in 1c2f437 But the mobile one doesn't: To resolve this, you'll have to start exporting the private APIs from the mobile version, just like it's done for the editor package: Let me know if that helps! |
|
Thanks for the advice, @tyxla. Oddly enough, this PR already exports private APIs for React Native. Additionally, a |
Ah, I missed that you already tried that. Okay, it seems like something is off with how the native version is handling imports. I think I've seen something similar and we fixed it by unlocking later, when all modules are properly loaded by RN. Will look for a reference. In the meantime I've pushed a couple of commits that aim to directly utilize It works in my local tests, let's see whether everything is green and let me know whether it works for you too. |
|
Looks like this resolved the mobile test issues as expected. |
|
Thank you! My local tests now also pass 👍 |
|
I'd also like to cc @ciampo and the rest of the @WordPress/gutenberg-components team since they maintain the |
ciampo
reviewed
Dec 8, 2023
packages/edit-site/src/components/global-styles/font-library-modal/collection-font-variant.js
Show resolved
Hide resolved
|
As a follow-up, if this PR is merged, I will also replace the proprietary |
tyxla
reviewed
Dec 11, 2023
|
Will leave the final approval to @ciampo since I've also contributed a fair amount to this PR 😅 |
ciampo
approved these changes
Dec 13, 2023
This was referenced Dec 14, 2023
|
Is components the right package for this? It's not a component? Nor really a component util but rather a very general util? |
I see your point. For more context, this decision was discussed in #56713 (comment) What package would be a better fit, in your opinion? |
|
I agree it's not the best fit, but neither was the block editor package before that. IMHO, ideally, we should either have a general utility package or use an external package, which could be what we do in the future with such utilities. I'm happy to move it to somewhere else if you feel there is a better fit. |
bph
removed
the
[Type] New API
|
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
artemiomorales
pushed a commit
that referenced
this pull request
Jan 4, 2024
…ark it as private API (#56758) * Components: Move kebabCase() function from block-editor package and mark it as private API * Update changelog * Fix native app test * Try to fix native app test * Try to fix mobile app test * Try importing kebabCase directly from components * Fix another import * Move changelog entry to Internal section * Change the argument type of kebabCase function to unknown * Fix lint error * Merge duplicate Internal sections * Remove type info in the JSDoc comment --------- Co-authored-by: Marin Atanasov <tyxla@abv.bg>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
See this discussion for information on why this PR was submitted: #56713 (comment)
What?
This PR moves the
kebabCasefunction, a utility function that exists in theblock-editorpackage, to thecomponentspackage and marks it as a private API.Why?
This function is used to separate words with hyphens. This function is equivalent to WP Core's _wp_to_kebab_case() function and also takes numbers into account.
On the other hand, the
kebabCase()function (originally theparamCase()function) imported from the externalchange-casepackage does not consider numbers. As a result, as reported in #55587, due to different kebab-case rules, there may be an issue where CSS variable names do not match.How?
To make the
kebabCase()function that the block editor package has available to many packages, including the component package itself, I moved it to the component package and marked it as a private API.For backwards compatibility, the block editor package's
kebabCase()function remains and internally references the component package's private API.Testing Instructions
Since I just moved the function, the logic remains the same and there should be no build errors or test failures.