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

Only show warning when overwrite existing preprocessor #34479

Closed

Conversation

Kudo
Copy link
Contributor

@Kudo Kudo commented Aug 23, 2022

Summary

from the original design of StyleSheet.setStyleAttributePreprocessor() in #11138, the overwriting warning shows when the existing preprocess is be overwritten. the behavior changes from 33b385825c72. This PR revises the logic back to original design.

Changelog

[Internal] [Fixed] - Show warning only when overwriting existing preprocessor in StyleSheet.setStyleAttributePreprocessor()

Test Plan

Unit Test

 PASS  Libraries/StyleSheet/__tests__/StyleSheet-test.js
  setStyleAttributePreprocessor
    ✓ should not show warning when set preprocessor first time (2 ms)
    ✓ should show warning when overwrite the preprocessor (1 ms)

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Contributor A React Native contributor. p: Expo Partner: Expo Partner labels Aug 23, 2022
@analysis-bot
Copy link

analysis-bot commented Aug 23, 2022

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,617,174 +1
android hermes armeabi-v7a 7,031,518 +3
android hermes x86 7,917,335 +2
android hermes x86_64 7,891,029 +1
android jsc arm64-v8a 9,494,870 +0
android jsc armeabi-v7a 8,272,579 +0
android jsc x86 9,432,763 +0
android jsc x86_64 10,025,808 +0

Base commit: 163171c
Branch: main

@Kudo Kudo marked this pull request as ready for review August 23, 2022 09:18
@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 Aug 23, 2022
@facebook-github-bot
Copy link
Contributor

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@Kudo Kudo force-pushed the setStyleAttributePreprocessor-warning branch from f380aec to e35e81c Compare August 25, 2022 01:34
@Kudo
Copy link
Contributor Author

Kudo commented Aug 25, 2022

also rebase this one for ci fixes

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 163171c
Branch: main

@facebook-github-bot
Copy link
Contributor

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @Kudo in 82ceb8c.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Aug 25, 2022
raykle pushed a commit to raykle/react-native that referenced this pull request Aug 27, 2022
Summary:
from the original design of `StyleSheet.setStyleAttributePreprocessor()` in facebook#11138, the overwriting warning shows when the existing preprocess is be overwritten.  the behavior changes from facebook@33b385825c72. This PR revises the logic back to original design.

## Changelog

[Internal] [Fixed] - Show warning only when overwriting existing preprocessor in `StyleSheet.setStyleAttributePreprocessor()`

Pull Request resolved: facebook#34479

Test Plan:
Unit Test

```
 PASS  Libraries/StyleSheet/__tests__/StyleSheet-test.js
  setStyleAttributePreprocessor
    ✓ should not show warning when set preprocessor first time (2 ms)
    ✓ should show warning when overwrite the preprocessor (1 ms)
```

Reviewed By: dmitryrykun

Differential Revision: D38940676

Pulled By: cipolleschi

fbshipit-source-id: 80cf30fff62f4a02c17f7f42b3260a6011d5fc82
dmytrorykun pushed a commit that referenced this pull request Sep 14, 2022
Summary:
from the original design of `StyleSheet.setStyleAttributePreprocessor()` in #11138, the overwriting warning shows when the existing preprocess is be overwritten.  the behavior changes from 33b385825c72. This PR revises the logic back to original design.

## Changelog

[Internal] [Fixed] - Show warning only when overwriting existing preprocessor in `StyleSheet.setStyleAttributePreprocessor()`

Pull Request resolved: #34479

Test Plan:
Unit Test

```
 PASS  Libraries/StyleSheet/__tests__/StyleSheet-test.js
  setStyleAttributePreprocessor
    ✓ should not show warning when set preprocessor first time (2 ms)
    ✓ should show warning when overwrite the preprocessor (1 ms)
```

Reviewed By: dmitryrykun

Differential Revision: D38940676

Pulled By: cipolleschi

fbshipit-source-id: 80cf30fff62f4a02c17f7f42b3260a6011d5fc82
Kudo added a commit to expo/expo that referenced this pull request Oct 5, 2022
…9318)

# Why

follow up #17138 (comment).
close ENG-4745

# How

the fix facebook/react-native#34479 is landed in react-native 0.70.1. now we can remove the workaround.

# Test Plan

ios unversioned expo go + NCL
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Contributor A React Native contributor. Merged This PR has been merged. p: Expo Partner: Expo Partner 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.

4 participants