Skip to content

Fix comma operator discarding shadow/textShadow options in preprocess - #2915

Merged
zoontek merged 3 commits into
necolas:masterfrom
charan-rathore:patch-1
Oct 21, 2026
Merged

zoontek merged 3 commits into
necolas:masterfrom
charan-rathore:patch-1

Conversation

@charan-rathore

@charan-rathore charan-rathore commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Problem

The two shadow guards in StyleSheet/preprocess.js use the comma operator:

if (
  (options.shadow === true,
  style.shadowColor != null || ...)
)

so options.shadow / options.textShadow are evaluated and discarded. The shadow* to boxShadow and textShadow* to textShadow conversions run whenever the legacy props are present, even when the caller explicitly disables them through the StyleSheet options. Nothing in src passes false today, so this bug has no visible effect for users; only direct calls to StyleSheet(styles, { shadow: false }) or preprocess(style) see a change.

Fix

Use && as intended, matching how StyleSheet/index.js opts in with defaultPreprocessOptions = { shadow: true, textShadow: true }. The existing shadow tests now pass that option explicitly (they previously relied on the comma bug), and new tests cover the disabled path: no options, or { shadow: false } / { textShadow: false }, removes the legacy props instead of converting them.

Fixes #2914

Tests

  • New disabled-path tests fail before the fix and pass after it.
  • jest packages/react-native-web/src/exports/StyleSheet: 91 passed, 51 snapshots.

AI-assisted; Charan Rathore is the human driver.

The two guards in StyleSheet/preprocess used the comma operator, so options.shadow and options.textShadow were evaluated and discarded: the shadow* to boxShadow and textShadow* to textShadow conversions ran whenever the legacy props were present, even when the caller disabled them through StyleSheet options.

Use && as intended and cover the disabled path with tests.

Fixes necolas#2914

Signed-off-by: Charan Rathore <[email protected]>
@codesandbox-ci

codesandbox-ci Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@zoontek

zoontek commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the fix!

Two corrections to the description:

  • customStyleq does not forward { shadow: false, textShadow: false }. It spreads defaultPreprocessOptions ({ shadow: true, textShadow: true }), and only the caller can override them. Nothing in src passes false today, so this bug has no visible effect for users. Only direct calls to StyleSheet(styles, { shadow: false }) or preprocess(style) see a change.
  • When the option is off, the legacy props are removed, not kept as they are. The loop still skips shadow* / textShadow*, which is what the new tests expect ({}). Can you update the wording?

@necolas The , comes from 2a901e5 (#2531), so options.shadow / options.textShadow were always ignored. Was && indeed involuntary, and is this fix OK?

Review nit from zoontek: match the spacing of the other describe blocks in this file.

Signed-off-by: Charan Rathore <[email protected]>
@charan-rathore

Copy link
Copy Markdown
Contributor Author

Thanks, zoontek! The nit is pushed and the description now has both corrections.

@zoontek
zoontek merged commit 7744567 into necolas:master Oct 21, 2026
6 checks passed
@zoontek

zoontek commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

Thanks, I'm merging this. I might make some additional changes depending on @necolas's response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

preprocess.js: broken if queries

2 participants