Migrate bare values to named values #18000
Merged
Conversation
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
This in theory can convert bare values to named values _if_ there exists a replacement. This is the minimal amount of work required to make that happen. But this also means that there is additional unnecessary work involved, such as: 1. Canonicalizing the value — we bare values don't even include whitespace-like characters so this is unnecessary work. 2. If we can't find a replacement using a named value, we will try arbitrary values... but it's already an arbitrary value. Will tackle these in the next few commits.
This will allow us to reuse them later in other migrations, once we split the arbitrary-utilities migration to also handle the bare value to named value migration.
This is now moved to its own dedicated migration file
| ? `${designSystem.theme.prefix}:${input}` | ||
| : input, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Don't we have exactly this implementation of parseCandidate in some other migration? I feel like I've seen this before.
There was a problem hiding this comment.
Ah yeah, I copied this whole file and deleted what I didn't need. Can move this to the candidates.ts file but should we just call it parseCandidate or something like prefixAwareParseCandidate`
There was a problem hiding this comment.
Just parseCandidate is fine for now. Can improve the name if we feel it's necessary later.
thecrypticace
approved these changes
May 13, 2025
This comment was marked as spam.
This comment was marked as spam.
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.
This PR improves the upgrade tool by also migrating bare values to named values defined in the
@theme. It's similar to the existing migration we already had, but specialized for just bare values. This makes it a bit faster as well because we don't have to try and replace a bare value with a bare value.Recently we shipped some updates dat allowed us to migrate arbitrary values (with square brackets), but we didn't migrate bare values yet.
That means that in this example:
We migrated this to:
With this change, we will also try and migrate the bare value to a named value. So this example:
Now becomes:
Test plan
Before:
After:
