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 StyleSheet.flatten returning undefined for falsy values #46294

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hazelmeow
Copy link

@hazelmeow hazelmeow commented Sep 2, 2024

Closes #46293.

Summary:

StyleSheet.flatten currently returns undefined when called with a falsy value (null/undefined/false/empty string). I ran into this with a conditional style that happened to pass false as the style to a component from react-native-gesture-handler. The library uses flatten to flatten the style prop and then copy some of the computed styles to another component, and assumed that flatten would always return a style object. flatten returned undefined, and I got a runtime error for trying to read properties of undefined.

The current behavior is not represented in the TypeScript types for the function. The docs say that flatten "flattens an array of style objects, into one aggregated style object", and also don't mention how it might return undefined. I think changing flatten to return an empty object for falsy values better matches the purpose and current usage of the function, and is a better option than widening the return type to include undefined or leaving this behavior undocumented.

Changelog:

[General] [Fixed] - Fix StyleSheet.flatten returning undefined for falsy values

Test Plan:

Tests pass with some changes:

  • One test was removed that expected flattenStyle(null) to be undefined ("should not allocate an object when there is no style"). Since this behavior isn't documented in the types or docs, I don't think it should still be considered the correct behavior. I also think the performance loss is probably very small and worth the correctness.
  • Another test was removed that expected flattenStyle(1234, null) to be undefined. This test originally expected it to return {}, but this was changed to undefined when ReactNativePropRegistry was removed in a8e3c7f. I think this test was left over from when StyleSheet.create returned opaque values, and can be removed.
  • A test was added for falsy values.
  • Some snapshots were updated (style prop changed from undefined to {}).

@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 Sep 2, 2024
@react-native-bot
Copy link
Collaborator

Fails
🚫

📋 Verify Changelog Format - See Changelog format

Generated by 🚫 dangerJS against 75c528a

@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Sep 2, 2024
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. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

StyleSheet.flatten with a falsy value returns undefined
3 participants