-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(xplat): Add support for nested selectors to Griffel to StyleX converter #30791
chore(xplat): Add support for nested selectors to Griffel to StyleX converter #30791
Conversation
…lat-griffel-stylex-nested-selectors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch on this one, I hadn't noticed the signature differences between stylex and griffel for these.
[atRule: `@${string}`]: string | number; | ||
type ValueWithSelectors = { | ||
default?: string | number | ValueWithSelectors | null; | ||
[pseudoClass: `:${string}`]: string | number | ValueWithSelectors; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, didn't realize index signatures could be added this way.
} else { | ||
throw new Error(`Unsupported nested selector: '${selector}' '${key}'`); | ||
console.warn(`Unsupported selector `, [...selectors, key]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I follow the logic now, we will emit warnings with unsupported selectors but not end up passing them to stylex. I think that is better than having the hard errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we'll need a longer-term solution to catch and fix these cases at development time, like a lint rule or something. But for now, this at least makes it so it doesn't crash with an invalid selector.
ℹ️ Note this is being merged into the
xplat
branchPrevious Behavior
The convertGriffelToStyleX platform adapter didn't support nested selectors.
New Behavior
The converter now adapts the griffel format of nested selectors:
into the stylex format of nested selectors: