[CLASS-14252] Scope RTL value-heuristics in recommended config to high-signal checks#3085
Draft
maddy531 wants to merge 4 commits into
Draft
[CLASS-14252] Scope RTL value-heuristics in recommended config to high-signal checks#3085maddy531 wants to merge 4 commits into
maddy531 wants to merge 4 commits into
Conversation
…-heuristics in recommended to high-signal checks only The `recommended` config enabled all six value-heuristics of `require-logical-properties-for-rtl`, but five of them have no logical-property fix and fire almost entirely on RTL-safe code, forcing `eslint-disable` suppressions on correct lines. Keep `warnBackgroundPosition` (real, fixable signal); disable `warnDirectionalTransforms`, `warnShadows`, `warnGradients`, `warnCursorDirections`, `warnBackgroundPositionXY`. This matches the rule's own `defaultOptions`. Part of CLASS-14252. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 4143ffa The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…emove low-signal RTL heuristics; expand padding/margin shorthand Remove the directional transform/shadow/gradient/cursor and backgroundPositionX/Y checks outright (including their options) rather than disabling them: they have no logical-property fix and are structurally unable to distinguish RTL-safe code from real bugs, so leaving them as off-by-default toggles would only keep dead, un-fixable checks around. Schema uses additionalProperties:false, so this is a breaking change for any config that set those options -> changeset bumped to major. Extend the padding/margin shorthand check to 3- and 4-value forms with autofix. The 4-value form is the genuine RTL hazard (left/right differ, don't auto-mirror) and was previously uncaught. Collapse the recommended config entry to bare "error" since the lone surviving option just restated defaultOptions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rop last rule option; autofix float/clear logical values Remove the final warnBackgroundPosition option so the rule takes no options at all -- the backgroundPosition/background directional check is now unconditional, matching float/clear/direction. In the audited cases it only ever caught real bugs, so a config-level off-switch wasn't justified; per-line eslint-disable remains the escape hatch. Add autofix for float: left/right -> inline-start/inline-end and clear likewise. The flow-relative float/clear values are now Baseline (Chrome/Edge 118, Safari 15, Firefox 55), so the fix the rule already recommended can be applied automatically. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
Size Change: -606 B (-0.47%) Total Size: 128 kB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (b0f8cde) and published all packages with changesets to npm. You can install the packages in ./dev/tools/deploy_wonder_blocks.js --tag="PR3085"Packages can also be installed manually by running: pnpm add @khanacademy/wonder-blocks-<package-name>@PR3085 |
Contributor
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-zzvfrbpwum.chromatic.com/ Chromatic results:
|
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
1 participant
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.
Summary
I previously had the 6 rtl lint rules set up for alter testing to see if they were helpful in catching RTL errors. During the transition to WB, I enabled all 6 rules as an experiment. While I was trying to set the eslint rule from warning to error in the frontend repo, I found that most for these rules didn't help.
This PR keeps the one one of these rules and removed the other 5
warnBackgroundPositioncalc()/percentageswarnDirectionalTransformstranslateX(-50%)centering (RTL-safe) from directional offsetswarnShadowswarnGradientswarnCursorDirectionse-resize/w-resize— trivial, rarewarnBackgroundPositionXYbackgroundPositionYis the block axis, which RTL never mirrorsStill fully enforced: the property-name → logical-property checks (
marginLeft→marginInlineStart, etc., with autofix),textAlign: left/right→start/end,float/clear, padding/margin shorthands, andbackgroundPositiondirectional detection. None of those are gated by these flags.Evidence
Audited the four KA
frontendPRs escalating this rule toerror(CLASS-14252). They introduce 25 neweslint-disablecomments:translateX(-50%)centering / counter-rotated orbittransparent→white→transparentgradientboxShadow(already paired with:dir(rtl))backgroundPositionY: bottom(block axis)backgroundPosition: bottom right/leftcalc()(kept enforced)23 of 25 (92%) are false positives eliminated by this change. The remaining 2 are caught by the check we keep, and are themselves fixable without a disable.
Risk
Disabling these means a genuinely directional transform/shadow/gradient wouldn't be linted. Across 25 real cases audited, zero were genuinely directional — the risk is empirically negligible, and the real directional cases (
backgroundPosition) stay covered.Test plan
recommendedconfig flags (verified — only the rule's own test file and unrelated codegen-template tests reference these names).require-logical-properties-for-rtlerrors after upgrading.🤖 Generated with Claude Code