Override designSystem methods by introducing a mockDesignSystem#14718
Closed
RobinMalfait wants to merge 3 commits into
Closed
Conversation
This only exists to implement a poor man's dependency injection system. We essentially want to reuse the existing `designSystem` as much as possible. But for some migrations we want to do things slightly differently (e.g.: not mutating arbitrary values) This function allows us to override some of the `designSystem` methods. Bonus points: overrode the `designSystem#parseCandidate` such that it doesn't do any caching anymore. This means that we can safely use `designSystem.parseCandidate` instead of using `parseCandidate(candidate, designSystem)`
Now that we added the `mockDesignSystem` where parsed candidates aren't cached anymore, it means that we will always get a fresh `candidate`. Therefore cloning that candidate AST again is not necessary anymore.
Member
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @RobinMalfait and the rest of your teammates on |
This was referenced Oct 18, 2024
Member
|
Going to close this in favor of #14720 — I think that's the cleaner way to approach this problem 👍 |
RobinMalfait
added a commit
that referenced
this pull request
Oct 18, 2024
This PR will optimize and simplify the candidates when printing the candidate again after running codemods. When we parse a candidate, we will add spaces around operators, for example `p-[calc(1px+1px)]]` will internally be handled as `calc(1px + 1px)`. Before this change, we would re-print this as: `p-[calc(1px_+_1px)]`. This PR changes that by simplifying the candidate again so that the output is `p-[calc(1px+1px)]`. In addition, if _you_ wrote `p-[calc(1px_+_1px)]` then we will also simplify it to the concise form `p-[calc(1px_+_1px)]`. Some examples: Input: ```html <div class="[p]:flex"></div> <div class="[&:is(p)]:flex"></div> <div class="has-[p]:flex"></div> <div class="px-[theme(spacing.4)-1px]"></div> ``` Output before: ```html <div class="[&:is(p)]:flex"></div> <div class="[&:is(p)]:flex"></div> <div class="has-[&:is(p)]:flex"></div> <div class="px-[var(--spacing-4)_-_1px]"></div> ``` Output after: ```html <div class="[p]:flex"></div> <div class="[p]:flex"></div> <div class="has-[p]:flex"></div> <div class="px-[var(--spacing-4)-1px]"></div> ``` --- This is alternative implementation to #14717 and #14718 Closes: #14717 Closes: #14718
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
2 participants
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.
We can definitely change the name, also open for suggestions here.
The idea is as follows:
designSystem(and therefore keep all the existing functionality)decodeArbitraryValuesfunction such that we don't add spaces around operators anymore. This is a divergence from the actual implementation. This allows us to keep the whitespace from the candidate without trying to be smart about it.&:is(…). We do get rid of this wrapper in thismockDesignSystemfunction as well. This has a downside that if people literally wrote[&:is(p)]:flexthat it will be converted to[p]:flexbut I think that's reasonable.Another thing that this unlocks is that by overriding the
parseCandidateandparseVariantfunctions, that we can remove the layer of caching. This means that we don't have to cache anymore, and even better, we don't have to usestructuredClone(candidate)anymore. We usedstructuredClonebefore to make sure that we didn't mutate existing candidates in the cached design system. But now everything is fresh.Some examples:
Input:
Output before:
Output after: