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 transform style processing for native #124

Merged
merged 1 commit into from
May 16, 2024
Merged

Fix transform style processing for native #124

merged 1 commit into from
May 16, 2024

Conversation

necolas
Copy link
Contributor

@necolas necolas commented May 16, 2024

For simplicity (and to fix an internal edge case), we now translate all 'transform' values to the React Native object syntax. If a string 'transform' is ever passed to an Animated component, the animation will now work as expected.

Close #123

Copy link

github-actions bot commented May 16, 2024

compressed-size: runtime library

Size change: -0.02 kB
Total size: 18.70 kB

Filename: gzip (minify) kB size kB change % change
./packages/react-strict-dom/dist/native/index.js 14.87 (46.93) -0.02 (-0.10) -0.1% (-0.2%) 🟢
View unchanged
Filename: gzip (minify) kB size kB change % change
./packages/react-strict-dom/dist/dom/index.js 2.89 (8.69) 0.00 (0.00) 0.0% (0.0%)
./packages/react-strict-dom/dist/dom/runtime.js 0.95 (2.33) 0.00 (0.00) 0.0% (0.0%)

Copy link

RSD benchmarks (for native)

Base c10e6a9

react-strict-dom@0.0.10 benchmark
node tests/benchmarks/benchmarks.node.js

Benchmark ops/sec deviation (%) samples
css.create() small style 5,329,572 0.53 95
css.create() small styles 2,754,094 1.78 92
css.create() big style 458,358 0.37 95
css.create() complex style 290,580 0.27 99
css.props() small style 905,726 0.20 96
css.props() large style 142,479 0.14 97
css.props() small merge 692,636 0.29 96
css.props() large merge 70,854 0.28 98

Patch aeb43f5

react-strict-dom@0.0.10 benchmark
node tests/benchmarks/benchmarks.node.js

Benchmark ops/sec deviation (%) samples
css.create() small style 5,343,449 0.39 96
css.create() small styles 2,545,385 0.31 97
css.create() big style 465,171 0.40 96
css.create() complex style 292,678 0.20 98
css.props() small style 893,608 0.21 97
css.props() large style 141,650 0.46 97
css.props() small merge 682,682 0.21 98
css.props() large merge 71,049 0.20 96

Copy link
Contributor

@nitishmehrotra870 nitishmehrotra870 left a comment

Choose a reason for hiding this comment

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

looks good to me.

For simplicity (and to fix an internal edge case), we now translate all
'transform' values to the React Native object syntax. If a string
'transform' is ever passed to an Animated component, the animation will
now work as expected.

Close #123
Copy link

RSD benchmarks (for native)

Base c10e6a9

react-strict-dom@0.0.10 benchmark
node tests/benchmarks/benchmarks.node.js

Benchmark ops/sec deviation (%) samples
css.create() small style 5,273,711 0.38 95
css.create() small styles 2,766,310 2.07 92
css.create() big style 454,157 0.92 95
css.create() complex style 289,828 0.83 96
css.props() small style 887,891 0.21 96
css.props() large style 141,513 0.90 99
css.props() small merge 672,482 0.43 94
css.props() large merge 71,241 0.13 98

Patch b9b7894

react-strict-dom@0.0.10 benchmark
node tests/benchmarks/benchmarks.node.js

Benchmark ops/sec deviation (%) samples
css.create() small style 5,254,477 0.44 97
css.create() small styles 2,878,754 0.30 96
css.create() big style 458,379 0.37 97
css.create() complex style 286,468 0.26 96
css.props() small style 905,983 0.20 99
css.props() large style 141,376 0.35 94
css.props() small merge 698,510 0.28 97
css.props() large merge 72,090 0.36 99

Comment on lines +508 to +511
{
"translateX": 1,
},
],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The loss of translateY here isn't technically a regression because the value is 1em and wouldn't have worked before. (And the other transform types aren't supported on React Native). However we could look into supporting non-px units for translate at some point in the future

@necolas necolas merged commit 13f1454 into main May 16, 2024
7 checks passed
@necolas necolas deleted the transform-fix branch May 17, 2024 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants