feat: memoize computed style to prevent unnecessary re-renders#36
Merged
Conversation
Collaborator
|
Can you update benchmark? |
Collaborator
Author
|
@exo-egor updated |
Collaborator
|
Test fails |
exo-egor
reviewed
Mar 24, 2026
exo-egor
reviewed
Mar 24, 2026
|
|
||
| const { getByTestId } = render(<AnimatedComponent />) | ||
| const element = getByTestId('foo') | ||
| expect(element.props.style).toEqual({ opacity: 0.5, flex: 1 }) |
Collaborator
There was a problem hiding this comment.
this seems like a bug or test can't find style?
Collaborator
Author
There was a problem hiding this comment.
this is same error on master outside of this pr scope
Collaborator
Author
There was a problem hiding this comment.
IDK about that. I checkout to master and run yarn test get the same error. maybe tests where not in ci before
➜ shakl git:(master) yarn test
yarn run v1.22.22
$ jest /test
PASS test/props.test.js
✓ accepts a style prop (7 ms)
✓ doesn't pass `theme` property to the child by default but passes to the style callback (1 ms)
✓ pass `theme` property to the style callback when using .attrs
✓ pass `theme` property to the style callback when using .extend (1 ms)
✓ pass `theme` property to the style callback when using .withChild (1 ms)
✓ pass `theme` property to the style callback when using .withComponent
✓ passes `theme` property if `omitProps` specified without it
PASS test/static.test.js
✓ allows providing defaultProps (7 ms)
PASS test/attrs.test.js
✓ creates a styled component with custom props (7 ms)
✓ creates a styled component with dynamic props (2 ms)
✓ creates a styled component with children prop
PASS test/basic.test.js
✓ creates a styled component (9 ms)
✓ creates a styled component with null static styles (1 ms)
✓ creates a styled component with null dynamic styles (1 ms)
✓ creates a styled component with empty static styles
✓ creates a styled component with empty dynamic styles (1 ms)
✓ creates a styled component with dynamic styles based on props
✓ creates a styled component of a functional component (2 ms)
✓ creates a styled component of a class component
PASS test/priority.test.js
✓ style prop should have a higher priority (9 ms)
✓ extended styles should have a higher priority (1 ms)
PASS test/dom.test.js
✓ creates a styled component (5 ms)
PASS test/name.test.js
✓ has proper display name (8 ms)
✓ allows providing a custom display name
✓ falls back to Component.name if Component.displayName cannot be inferred (1 ms)
✓ keeps custom display name when extended (1 ms)
✓ exposed primitives have proper display names (4 ms)
PASS test/children.test.js
✓ passes a single child properly (9 ms)
✓ passes multiple children properly (2 ms)
PASS test/ref.test.js
✓ forwards ref to wrapped component (10 ms)
✓ forwards ref created with React.createRef() to wrapped component (1 ms)
PASS test/child.test.js
✓ creates a styled component with a child (12 ms)
✓ creates a styled component with a child with custom props (1 ms)
✓ creates a styled component with a child with dynamic custom props (2 ms)
✓ attaches a ref to a child (1 ms)
PASS test/animated.test.js
✓ works with animated components (13 ms)
PASS test/primitives.test.js
✓ exposes primitives (5 ms)
✓ exposed primitives are valid (10 ms)
PASS test/extend.test.js
✓ extends a styled component with extend() (14 ms)
✓ extends a styled component with styled(Styled) (4 ms)
✓ extends a styled component with null static styles (1 ms)
✓ extends a styled component with null dynamic styles (2 ms)
✓ extends a styled component with empty static styles
✓ extends a styled component with empty dynamic styles
✓ extends a styled component with dynamic styles based on props (1 ms)
✓ extends a styled component with custom props (5 ms)
PASS test/stylesheet.test.js
✓ works with stylesheet styles (19 ms)
✓ works with stylesheet styles when use array (1 ms)
PASS test/example.test.js
✓ works (5 ms)
FAIL test/reanimated.test.js
✕ works with reanimated styles (22 ms)
✕ works with reanimated styles when use array (2 ms)
✕ works with reanimated styles when use array and some style is undefined (1 ms)
● works with reanimated styles
expect(received).toEqual(expected) // deep equality
Expected: {"flex": 1, "opacity": 0.5}
Received: [{"flex": 1, "opacity": 0}, {"opacity": 0.5}, {}]
18 | const { getByTestId } = render(<AnimatedComponent />)
19 | const element = getByTestId('foo')
> 20 | expect(element.props.style).toEqual({ opacity: 0.5, flex: 1 })
| ^
21 | expect(element).toHaveAnimatedStyle({ opacity: 0.5 })
22 | })
23 |
at Object.toEqual (test/reanimated.test.js:20:31)
● works with reanimated styles when use array
expect(received).toEqual(expected) // deep equality
Expected: {"flex": 1, "opacity": 0.5, "width": 50}
Received: [{"flex": 1, "opacity": 0}, {"opacity": 0.5}, {"width": 50}, {}]
35 | const { getByTestId } = render(<AnimatedComponent />)
36 | const element = getByTestId('foo')
> 37 | expect(element.props.style).toEqual({ opacity: 0.5, flex: 1, width: 50 })
| ^
38 | expect(element).toHaveAnimatedStyle({ opacity: 0.5 })
39 | })
40 | it('works with reanimated styles when use array and some style is undefined', () => {
at Object.toEqual (test/reanimated.test.js:37:31)
● works with reanimated styles when use array and some style is undefined
expect(received).toEqual(expected) // deep equality
Expected: {"flex": 1, "opacity": 0.5, "width": 50}
Received: [{"flex": 1, "opacity": 0}, {"opacity": 0.5}, {"width": 50}, {}]
51 | const { getByTestId } = render(<AnimatedComponent />)
52 | const element = getByTestId('foo')
> 53 | expect(element.props.style).toEqual({ opacity: 0.5, flex: 1, width: 50 })
| ^
54 | expect(element).toHaveAnimatedStyle({ opacity: 0.5 })
55 | })
56 |
at Object.toEqual (test/reanimated.test.js:53:31)
Test Suites: 1 failed, 15 passed, 16 total
Tests: 3 failed, 49 passed, 52 total
Snapshots: 70 passed, 70 total
Time: 1.544 s, estimated 2 s
Ran all test suites matching /\/test/i.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
➜ shakl git:(master)
Collaborator
There was a problem hiding this comment.
seems like issue in reanimated version, i'll create pr
Collaborator
Author
There was a problem hiding this comment.
can we go forward with merge?
Collaborator
|
Depends on #37 |
Collaborator
Author
|
@exo-egor should I discard test changes here? |
2c024d0 to
4ea42e0
Compare
Collaborator
|
yes needs rebas |
Collaborator
Author
|
@exo-egor done |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Test plan