Repository navigation
[babel-plugin] Fix paddingInline not resetting paddingInlineStart/End in application-order - #2020
Draft
henryqdineen wants to merge 1 commit into
Draft
henryqdineen wants to merge 1 commit into
henryqdineen wants to merge 1 commit into
Conversation
With styleResolution: 'application-order', applying `padding` or `paddingInline` after `paddingInlineStart`/`paddingInlineEnd` left the earlier longhand's class in the className, and because it is emitted at higher specificity it won the cascade over the later shorthand. The `paddingInline` expansion nulled `paddingStart`/`paddingEnd`, what we assume is a typo for `paddingInlineStart`/`paddingInlineEnd` (`marginInline` and the legacy `paddingInline` expansion already use the correct names). `padding` reuses this expansion, so both are fixed. Fixes meta#2018 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@henryqdineen is attempting to deploy a commit to the Meta Open Source Team on Vercel. A member of the Team first needs to authorize it. |
meta-cla
Bot
added
the
CLA Signed
This branch has not been deployed
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
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.
What changed / motivation ?
With
styleResolution: 'application-order', a laterpaddingorpaddingInlinedidn't override an earlierpaddingInlineStart/paddingInlineEnd. Both classes stayed in the resolvedclassName, and because the longhand is emitted at higher specificity, the earlier value won.The cause is what we assume is a typo in the
paddingInlineexpansion inapplication-order.js: it resetpaddingStart/paddingEndinstead ofpaddingInlineStart/paddingInlineEnd.marginInline, and thepaddingInlineexpansion inlegacy-expand-shorthands.js, already use the correct names.paddingreuses thepaddingInlineexpansion, so this one change fixes both.HubSpot uses
application-order, which is why I picked this up. We haven't hit it in practice, but it's an easy pattern to fall into (a base style withpaddingInlineEnd, overridden by apaddingreset).Linked PR/Issues
Fixes #2018
Additional Context
Tests added in
__tests__/legacy/stylex-transform-call-test.js:paddingandpaddingInlineapplied afterpaddingInlineStartand afterpaddingInlineEndnow resolve to only the shorthand's classmargin/marginInline, which already behaved correctly, as a comparisonExisting snapshots updated:
stylex call with short-form property collisionsand... with nullhad the bug in their expected output (e.g."x2vl965 x1i3ajwb"keptpadding-inline-end:10pxafter a laterpadding:2px); they now resolve to"x1i3ajwb xe2zdcy"and"x1i3ajwb"stylex.createsnapshots change which hashed keyspaddingsets tonull, because the reset now targets the correct propertiesI also checked the other expansions in
application-order.jsfor the same mistake (resetting a name that only exists as an alias) and found no others.Pre-flight checklist
🤖 Generated with Claude Code