Skip to content

Fix length validators wrongly accepting comma-prefixed values#1741

Open
durvesh1992 wants to merge 1 commit into
facebook:mainfrom
durvesh1992:fix/length-validators-reject-comma-prefix
Open

Fix length validators wrongly accepting comma-prefixed values#1741
durvesh1992 wants to merge 1 commit into
facebook:mainfrom
durvesh1992:fix/length-validators-reject-comma-prefix

Conversation

@durvesh1992

Copy link
Copy Markdown

Summary

The ESLint valid-styles rule accepts malformed CSS lengths like ',4px' and ',4rem'. Both length validators build their regex with the character class [-,+], which includes a literal comma:

// packages/@stylexjs/eslint-plugin/src/rules/isAbsoluteLength.js
new RegExp(`^([-,+]?\\d+(\\.\\d+)?${unit})$`)
//          ^^^^^ the comma here is matched literally

So a leading comma is accepted as if it were a sign. This changes [-,+] to [+-] in both isAbsoluteLength.js and isRelativeLength.js, allowing only an optional leading sign. Legitimate values (4px, -4px, +4px, 4rem, …) are unaffected.

This is the same defect fixed for the percentage validator in #1574, which did not touch the two length files — so this complements that PR rather than overlapping it.

Test plan

Added regression tests in stylex-valid-styles-test.js using textUnderlineOffset (a length-only property, so the length validators are exercised directly rather than the non-numeric string fallback that properties like height allow).

  • Before the fix: the new ',4px' / ',4rem' cases raise 0 errors (bug) → tests fail.
  • After the fix: they correctly error → tests pass.
  • Full @stylexjs/eslint-plugin suite: 549 passed, 9 suites, no regressions.
yarn --cwd packages/@stylexjs/eslint-plugin jest

isAbsoluteLength and isRelativeLength built their regex with the
character class [-,+], which includes a literal comma. As a result the
`valid-styles` ESLint rule accepted malformed lengths such as ',4px'
and ',4rem'.

Change [-,+] to [+-] in both validators so only an optional leading
sign is allowed. Legitimate values (4px, -4px, +4px, 4rem, ...) still
validate.

This is the same defect fixed for the percentage validator in facebook#1574,
which did not touch the two length files.

Added regression tests (textUnderlineOffset is length-only) that fail
before the change and pass after; full eslint-plugin suite is green.
@vercel

vercel Bot commented Jun 26, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
stylex Skipped Skipped Jun 26, 2026 9:43pm

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant