Skip to content
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

Fix regression in StyleSheet.setStyleAttributePreprocessor #22262

Closed
wants to merge 2 commits into from

Conversation

brentvatne
Copy link
Collaborator

This regression was caused by 199c918 - property values are initialized to true rather than a string that matches the property name now.

Test Plan:

fontFamily is not a valid style attribute
- node_modules/react-native/Libraries/StyleSheet/StyleSheet.js:347:23 in setStyleAttributePreprocessor

With this patch, this error is resolved.

Changelog:

  • [General] [Fixed] - Update StyleSheet.setStyleAttributePreprocessor for new format of ReactNativeStyleAttributes.

…199c918

Property values are initialized to true rather than a string that matches the
property name now.
@facebook-github-bot facebook-github-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 Nov 13, 2018
@react-native-bot react-native-bot added Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. 🔶APIs API: StyleSheet labels Nov 13, 2018
@janicduplessis
Copy link
Contributor

👍

@facebook-github-bot shipit

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Nov 13, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@janicduplessis is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@elicwhite
Copy link
Member

Yikes. setStyleAttributePreprocessor is sketchy. Thanks for catching this. Should definitely be handing it being true. The attributes it is checking is what React uses to know how to process host element's props and thus we want those objects to be as small as possible which is why they use true.

@react-native-bot
Copy link
Collaborator

@brentvatne merged commit 0408533 into facebook:master.

@facebook facebook locked as resolved and limited conversation to collaborators Nov 14, 2018
@react-native-bot react-native-bot added the Merged This PR has been merged. label Nov 14, 2018
@brentvatne
Copy link
Collaborator Author

Yikes. setStyleAttributePreprocessor is sketchy

it is a bit, but we do this kind of processing internally so I think there needs to be some way to expose the same power to users. I actually originally added this code iirc but definitely open to ideas for other ways to solve the use case mentioned in op

kelset pushed a commit that referenced this pull request Nov 26, 2018
Summary:
This regression was caused by 199c918 - property values are initialized to true rather than a string that matches the property name now.
Pull Request resolved: #22262

Differential Revision: D13048839

Pulled By: hramos

fbshipit-source-id: 09471334c37f3930aae7e35066943f33f8e617e5
@zpao zpao deleted the setStyleAttributePreprocessor-fix branch January 31, 2019 01:55
@hramos hramos removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 6, 2019
t-nanava pushed a commit to microsoft/react-native-macos that referenced this pull request Jun 17, 2019
…22262)

Summary:
This regression was caused by 199c918 - property values are initialized to true rather than a string that matches the property name now.
Pull Request resolved: facebook#22262

Differential Revision: D13048839

Pulled By: hramos

fbshipit-source-id: 09471334c37f3930aae7e35066943f33f8e617e5
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API: StyleSheet CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Merged This PR has been merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants