Upgrade: Migrate JS theme configuration keys with dot and slash in the property name #14736
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
Member
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @philipp-spiess and the rest of your teammates on |
philipp-spiess
force-pushed
the
10-21-upgrade_migrate_js_theme_configuration_keys_with_dot_and_slash_in_the_property_name
branch
from
06b6d11 to
19e512d
Compare
adamwathan
reviewed
Oct 21, 2024
Member
adamwathan
left a comment
adamwathan
left a comment
There was a problem hiding this comment.
Should we just be using CSS.escape or whatever on all JS stuff when converting it to CSS? There’s tons of other characters someone could be using in their JS config that need to be escaped when converted to CSS as well, like percentage signs for example.
philipp-spiess
force-pushed
the
10-21-upgrade_migrate_js_theme_configuration_keys_with_dot_and_slash_in_the_property_name
branch
from
19e512d to
ddcebfb
Compare
philipp-spiess
changed the base branch from
next
to
10-21-escape_js_theme_configuration_keys
philipp-spiess
force-pushed
the
10-21-escape_js_theme_configuration_keys
branch
from
fb06b4c to
99072ca
Compare
philipp-spiess
force-pushed
the
10-21-upgrade_migrate_js_theme_configuration_keys_with_dot_and_slash_in_the_property_name
branch
from
ddcebfb to
e9c9f7e
Compare
philipp-spiess
force-pushed
the
10-21-escape_js_theme_configuration_keys
branch
from
99072ca to
f99717b
Compare
philipp-spiess
force-pushed
the
10-21-upgrade_migrate_js_theme_configuration_keys_with_dot_and_slash_in_the_property_name
branch
from
e9c9f7e to
47f75e8
Compare
philipp-spiess
commented
Oct 22, 2024
thecrypticace
approved these changes
Oct 22, 2024
Member
Author
Merge activity
|
…grade_migrate_js_theme_configuration_keys_with_dot_and_slash_in_the_property_name
RobinMalfait
changed the base branch from
10-21-escape_js_theme_configuration_keys
to
graphite-base/14736
adamwathan
reviewed
Oct 22, 2024
adamwathan
reviewed
Oct 22, 2024
Comment on lines
125
to
134
| let property = keyPathToCssProperty([key[0]]) | ||
| if (property !== null) { | ||
| css += ` --${escape(property)}-*: initial;\n` | ||
| } | ||
| } | ||
|
|
||
| css += ` --${keyPathToCssProperty(key)}: ${value};\n` | ||
| let property = keyPathToCssProperty(key) | ||
| if (property !== null) { | ||
| css += ` --${escape(property)}: ${value};\n` | ||
| } |
Member
There was a problem hiding this comment.
It might be wise to prepend the -- before running escape here, because certain characters need to be escaped if they are the very first character but don't need to be escaped if they are not:
adamwathan
deleted the
10-21-upgrade_migrate_js_theme_configuration_keys_with_dot_and_slash_in_the_property_name
branch
philipp-spiess
added a commit
that referenced
this pull request
Oct 23, 2024
…e property name (#14736) This PR fixes an issue where JS configuration theme properties with dots or slashes in them would not migrate correctly. E.g.: ```ts import { type Config } from 'tailwindcss' module.exports = { theme: { width: { 1.5: '0.375rem', '1/2': '50%', } } } ``` This should convert to: ```css @theme { --width-1_5: 0.375rem; --width-1\/2: 50%; } ``` _Note: We will likely change the `--width-1_5` key to `--width-1\.5` in a follow-up PR._ --------- Co-authored-by: Robin Malfait <[email protected]> Co-authored-by: Adam Wathan <[email protected]> Co-authored-by: Adam Wathan <[email protected]>
Member
Author
|
Merged into |
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 fixes an issue where JS configuration theme properties with dots or slashes in them would not migrate correctly. E.g.:
This should convert to:
Note: We will likely change the
--width-1_5key to--width-1\.5in a follow-up PR.