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 react 18 failing test #897

Open
redonkulus opened this issue Jul 15, 2022 · 5 comments
Open

Fix react 18 failing test #897

redonkulus opened this issue Jul 15, 2022 · 5 comments

Comments

@redonkulus
Copy link
Contributor

During React 18 testing one unit test was not passing, further debugging is required. The package was published to unblock users from upgrading to React 18. This bug could break your application!

Issue when running the test:

  ● Sticky › should release when height gets changed (long Sticky)

    expect(received).toBe(expected) // Object.is equality

    Expected: "translate3d(0,1068px,0)"
    Received: "translate3d(0,-432px,0)"

       97 |     const style = t._style || t.style;
       98 |     expect(style.width).toBe('100px');
    >  99 |     expect(style.transform).toBe('translate3d(0,' + pos + 'px,0)');
          |                             ^
      100 |     expect(style.position).toBe('relative');
      101 |     expect(style.top).toBe('');
      102 | }

      at toBe (tests/unit/Sticky.test.js:99:29)
      at Object.shouldBeReleasedAt (tests/unit/Sticky.test.js:472:9)
@RainGrid
Copy link

RainGrid commented Apr 8, 2023

The library expects a DOM commit between two setState calls to calc position correct, but React 18 batches those two renders into one commit
For those who tries to fix dynamic height sticky for example: you can try

stickyInstance.updateInitialDimension();  
stickyInstance.forceUpdate(() => stickyInstance.update());

or

stickyInstance.updateInitialDimension();  
flushSync(() => stickyInstance.update());

@redonkulus
Copy link
Contributor Author

@RainGrid do you know the proper way to get access to the instance to force the update?

@RainGrid
Copy link

@RainGrid do you know the proper way to get access to the instance to force the update?

const stickyRef = useRef(null);
...
const stickyInstance = stickyRef.current;
...
<Sticky ref={stickyRef} >
...

@redonkulus
Copy link
Contributor Author

@RainGrid that works for hooks, but sticky-node is a class-based component. We will need a non-hook approach.

@RainGrid
Copy link

@redonkulus you can call this.forceUpdate(); inside the Sticky component
Outside you can pass the ref created by React.createRef() https://legacy.reactjs.org/docs/refs-and-the-dom.html#creating-refs. This works somehow, you will get an instance of Sticky with all methods

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants