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

play function does not work after React 18 #18258

Closed
reinertisa opened this issue May 19, 2022 · 66 comments
Closed

play function does not work after React 18 #18258

reinertisa opened this issue May 19, 2022 · 66 comments

Comments

@reinertisa
Copy link

reinertisa commented May 19, 2022

Screen Shot 2022-05-18 at 6 54 57 PM

**Describe the bug** I created stories for react-hook-form validation and used play function. It worked well React 17. After I updated React from 17 to 18, play function does not trigger validation. Do I miss something? @yannbf
export const ValidationErrors = {
  render: (args) => (
    <>
      <h4>Superstruct validation</h4>
      <Form
        defaultValues={{
          radioField1: null,
        }}
        resolver={superstructResolver(RadioGroupStruct)}
        onSubmit={action('Superstruct submit')}
        mode="onSubmit"
      >
        <FormRadioGroup
          name="radioField1"
          label="Trigger validation"
          options={['One', 'Two', 'Three', 'Four']}
          {...args}
        />
        <FormSubmit>Save</FormSubmit>
      </Form>
      
      <h4 className="mt-4">RHF validation</h4>
      <Form
        defaultValues={{
          radioField1: null,
        }}
        onSubmit={action('RHF submit')}
        mode="onSubmit"
      >
        <FormRadioGroup
          name="radioField1"
          label="Trigger validation"
          options={['One', 'Two', 'Three', 'Four']}
          {...args}
          validation={{validate: atLeastOne}}
        />
        <FormSubmit>Save</FormSubmit>
      </Form>
    </>

  ),
  args: {
    readOnly: false,
    disabled: false,
    help: 'help text here',
  },
  play: async ({canvasElement}) => {
    const btnElems = within(canvasElement).getAllByRole('button', {name: /Save/i});

    // Superstruct validation: trigger radio group
    await userEvent.click(btnElems[0]);

    // RHF validation: trigger radio group
    await userEvent.click(btnElems[1]);
  },
};

Expected output
Screen Shot 2022-05-18 at 6 57 47 PM

@shilman
Copy link
Member

shilman commented May 24, 2022

Do you a have a reproduction repo you can share? If not, can you create one? See how to create a repro. Thank you! 🙏

@reinertisa
Copy link
Author

@shilman and @yannbf Sorry for the delay. I created repo and story is under src directory. App.stories.js -> https://github.com/reinertisa/my-example-reproduction

Thank you for your help.

@reinertisa
Copy link
Author

@shilman and @yannbf I converted React version from 18 to 17.0.2 and I run storybook and validation works with play function great but with version 18, it does not work. If you need another repo for 17.0.2, I can create it if you want.

@shilman
Copy link
Member

shilman commented Jun 25, 2022

HI @reinertisa !! Thanks so much for creating a reproduction. However, I'm seeing the same behavior when I run your repro in both React 17 and 18:

image

Can you please help clarify what I'm supposed to see differently? Also can you share the output of npx storybook info for me on your dev machine?

@reinertisa
Copy link
Author

reinertisa commented Jun 25, 2022

Hi @shilman I see your screenshot for React 18 but When I use React 17. I see this screenshot. (I see validation error as expected)
Screen Shot 2022-06-24 at 5 51 32 PM
Screen Shot 2022-06-24 at 6 02 47 PM

@reinertisa
Copy link
Author

@shilman this is output for my dev machine. Thank you for your help.
Environment Info:

System:
OS: macOS 12.4
CPU: (8) x64 Intel(R) Core(TM) i7-6920HQ CPU @ 2.90GHz
Binaries:
Node: 16.13.2 - ~/.nvm/versions/node/v16.13.2/bin/node
Yarn: 3.2.1 - ~/.nvm/versions/node/v16.13.2/bin/yarn
npm: 8.1.2 - ~/.nvm/versions/node/v16.13.2/bin/npm
Browsers:
Chrome: 103.0.5060.53
Firefox: 94.0.2
Safari: 15.5
npmPackages:
@storybook/addon-actions: ^6.5.9 => 6.5.9
@storybook/addon-docs: ^6.5.9 => 6.5.9
@storybook/addon-essentials: ^6.5.9 => 6.5.9
@storybook/addon-interactions: ^6.5.9 => 6.5.9
@storybook/addon-links: ^6.5.9 => 6.5.9
@storybook/builder-webpack4: ^6.5.9 => 6.5.9
@storybook/manager-webpack4: ^6.5.9 => 6.5.9
@storybook/react: ^6.5.9 => 6.5.9
@storybook/testing-library: ^0.0.13 => 0.0.13

@shilman
Copy link
Member

shilman commented Jun 25, 2022

@reinertisa Clearly I need more coffee this morning! ☕

Ok, I'm able to reproduce the issue now (though intermittently -- sometimes I see the validation error in React18 also!)

Can you please update your story code like this and verify that the problem goes away?

const sleep = (ms) => new Promise((r) => setTimeout(r, ms));

export const Default = {
  render: (args) => <App {...args} />,
  play: async ({ canvasElement }) => {
    await sleep(0);
    await userEvent.click(within(canvasElement).getByRole('button', { name: /Submit/i }));
  },
};

It's not a proper fix, but if it works for you that will help us diagnose the problem and work towards a proper solution. Thanks!

@reinertisa
Copy link
Author

reinertisa commented Jun 25, 2022

@shilman Thank you for information but I used sleep function after we updated React 18 to see validation errors and it worked but after that our chromatic build failed. We are not sure this sleep function caused that problem. I hope you can find proper solution for it. I am looking forward to it. Also I need coffee. Enjoy ☕

@reinertisa
Copy link
Author

@shilman maybe you need to see chromatic failure link for your reference. When I use sleep function, it works locally but fails in chromatic. I can provide link to failure for your reference. https://www.chromatic.com/test?appId=5eeab205fdb1e500227646e6&id=62b0bbe8c5862a6e55a3e9d8
If you need anything else from me, please let me know. Thank you so much.

@shilman
Copy link
Member

shilman commented Jun 25, 2022

Thanks so much @reinertisa. No idea what the problem is yet, but this is a great starting point!

@IanVS
Copy link
Member

IanVS commented Sep 20, 2022

I can also confirm I'm experiencing this (or a very similar) issue after updating to react 18 (and storybook 7-alpha.33). I have the validation error problem as shown above, but also the first character is sometimes lost from await userEvent.type();. For instance:

await userEvent.type(input, '1234');

Results in 234 in the field. But if I add a 1ms sleep, then it's fine, and the whole thing is typed correctly.

Similarly, a "combobox" of mine is not opening correctly when first clicked on, unless I add a sleep.

How does the play function know when to fire? IOW, where does storybook trigger the play function, and is there a chance that it needs to wait just a bit longer for react to finish rendering?

@brycnguyen
Copy link

@reinertisa Clearly I need more coffee this morning! ☕

Ok, I'm able to reproduce the issue now (though intermittently -- sometimes I see the validation error in React18 also!)

Can you please update your story code like this and verify that the problem goes away?

const sleep = (ms) => new Promise((r) => setTimeout(r, ms));

export const Default = {
  render: (args) => <App {...args} />,
  play: async ({ canvasElement }) => {
    await sleep(0);
    await userEvent.click(within(canvasElement).getByRole('button', { name: /Submit/i }));
  },
};

It's not a proper fix, but if it works for you that will help us diagnose the problem and work towards a proper solution. Thanks!

This saved me today. Thanks! I'm also seeing this on my copy using react 18.

@sballesteros
Copy link

I think that I have noticed smtg similar with the latest preact as well. I think Preact changed some of the internal (maybe related to 10.10). I will try to create a repro later when I have time (I am kind of lazily hoping that a preact update fixes it).

@gkadillak-lyra
Copy link

@shilman is there a timeline for this fix? The workaround does help, but we've had to use sleep(1000) to get the tests to reliably run and this time is starting to stack up.

@IanVS
Copy link
Member

IanVS commented Nov 29, 2022

I see that @yannbf is assigned to this ticket, I think he might be the best to discuss the plans for it. I do hope it can be fixed soon, since it's essentially blocking my upgrade to react 18 right now.

@yannbf
Copy link
Member

yannbf commented Nov 30, 2022

@tmeasday I thought #18737 and #19125 fixed that issue, but I see they got released before alpha.33 and the issue is still happening 🤔

@IanVS
Copy link
Member

IanVS commented Jan 11, 2023

Wild, it fails for me!

Screen.Recording.2023-01-10.at.9.18.56.PM.mov

@bluebill1049
Copy link

I did change the code to just use handleSubmit without e.preventDefault

Screenshot 2023-01-11 at 1 33 10 pm

@IanVS
Copy link
Member

IanVS commented Jan 11, 2023

I see, thanks. If I make the callback to handleSubmit async, then I seem to have a race condition where sometimes after a refresh the test will pass, and sometimes it will fail...

@bluebill1049
Copy link

Do we have something like waitFor in action play?

@IanVS
Copy link
Member

IanVS commented Jan 11, 2023

Yes we can use that. Do you know what we could potentially trigger off of, though?

@bluebill1049
Copy link

handleSubmit is async by default, because it allows the user to do async action inside and also means there will be running micro task running inside the function itself, which may result in the issue itself.

@sw-tracker
Copy link

Any updates on this? We just upgraded to react 18 and we are encountering this issue. We have a sleep(1000) for now, but it would be nice to have a proper fix.

@IanVS
Copy link
Member

IanVS commented Apr 14, 2023

@sw-tracker if you have to sleep that long, it may be a different issue. I've found that sleeping for 1 ms is usually enough for this particular issue. I still haven't gotten to the root cause, and have resorting to sprinkling a few sleep(1) here and there. 😞

@sw-tracker
Copy link

@IanVS sorry yes, sleep(1) makes it work locally on my MacBook. I had to increase it to sleep(1000) because the Jenkins where I work is slow and 1ms doesnt suffice sometimes.

@yannbf
Copy link
Member

yannbf commented Apr 21, 2023

Can someone provide a simple reproduction with the latest Storybook 7? Thank you so much <3

@IanVS
Copy link
Member

IanVS commented Apr 21, 2023

@yannbf I updated my reproduction above to 7.0.6: stackblitz.com/edit/github-z8cetq?file=src/reacthookform.jsx.

@tmeasday
Copy link
Member

tmeasday commented May 4, 2023

@IanVS interestingly that reproduction itself is very intermittent--basically doesn't happen for me apart from the very first render.

Would it be useful to put together a non-SB reproduction @bluebill1049? Have you tried that @IanVS?

@tmeasday
Copy link
Member

tmeasday commented Jun 6, 2023

@IanVS do you know what the status is on this one? I'm wondering if we should close this issue as ultimately it doesn't seem like an issue with SB itself.

@IanVS
Copy link
Member

IanVS commented Jun 6, 2023

I've resorted to sprinkling sleep(1) throughout stories as needed. I agree that it doesn't seem to be an issue in Storybook. Storybook is just executing so fast that it's surfacing issues in react-hook-form, most likely. Though I'm curious why that wouldn't also happen in other testing tools (presumably).

@tmeasday
Copy link
Member

tmeasday commented Jun 7, 2023

Ok, I had a play around with the reproduction on this issue outside of SB.

I reproduced the behaviour with @testing-library/user-event@13.x (which is what @storybook/testing-library uses). Upgrading to 14.0.0 resolves the issue. I'm not sure what's changed (unclear from changelogs), but sounds like we should upgrade our deps to improve the situation:

https://codesandbox.io/s/dry-fire-bmoc2t?file=/src/App.js

cc @yannbf

@jtstothard
Copy link

Ok, I had a play around with the reproduction on this issue outside of SB.

I reproduced the behaviour with @testing-library/user-event@13.x (which is what @storybook/testing-library uses). Upgrading to 14.0.0 resolves the issue. I'm not sure what's changed (unclear from changelogs), but sounds like we should upgrade our deps to improve the situation:

https://codesandbox.io/s/dry-fire-bmoc2t?file=/src/App.js

cc @yannbf

This would be great but I think testing library user event 14 was tested and reverted because of incompatibility, it would be great if this is now possible though!

storybookjs/testing-library#19

@tobeycodes
Copy link

tobeycodes commented Jul 18, 2023

I have started to experience this after upgrading React 17 to 18. In our case we have a useEffect that runs to update state to hide the sidebar.

const [isOpen, setIsOpen] = useState(false);

useEffect(() => {
  if(hasSomeCondition) {
    setIsOpen(false);
  }
}, [hasSomeCondition[)

Our test has the following which the element has onClick={() => setIsOpen(true)}

await waitFor(() => userEvent.click(canvas.getByTestId('navtoggle')));

What is happening for us is that the user click event is happening before the useEffect has run when the component renders. Adding a sleep above solves the problem as the click will run after the useEffect has run.

@reinertisa
Copy link
Author

@shilman hi, today I updated storybook/testing-library (0.0.13 -> 0.2.0(latest)) and removed sleep function from play and all interactions triggered react-hook-form validations as expected locally. By the way we are still using storybook(6.5.16). Did you have any chance test this issue with latest storybook/testing-library(0.2.0) ? Maybe this is the solution. But when we tried to catch snapshot using chromatic, all snapshots are failed and this is the screenhot coming from chromatic build.
Screenshot 2023-08-30 at 1 07 40 AM

@yannbf
Copy link
Member

yannbf commented Aug 30, 2023

@shilman hi, today I updated storybook/testing-library (0.0.13 -> 0.2.0(latest)) and removed sleep function from play and all interactions triggered react-hook-form validations as expected locally. By the way we are still using storybook(6.5.16). Did you have any chance test this issue with latest storybook/testing-library(0.2.0) ? Maybe this is the solution. But when we tried to catch snapshot using chromatic, all snapshots are failed and this is the screenhot coming from chromatic build.

Hey there! As mentioned a few times in this thread, it seems like the issue is with @testing-library/user-event version 13.

I would highly recommend you to upgrade to Storybook 7 if you can, not only because Storybook 6 is about 3 years old by now, but also because there were many fixes regarding the interactions addon and its relation to @storybook/testing-library. Also, another side effect which might impact your project is that @storybook/testing-library v0.2.0 uses @storybook/instrumenter version 7, which should be used with Storybook 7.

Here's a video comparing two repros, one that fails and the other one that does not:
Storybook 7 + @storybook/testing-library 0.0.14 - fails
Storybook 7 + @storybook/testing-library 0.2.0 - works

testing.library.issue.mov

I also noticed that your error does not seem to be the same as the ones mentioned in the reproduction:

image

This error happens when people use @testing-library/react instead @storybook/testing-library in their story files. Is that a possibility in your project?

@reinertisa
Copy link
Author

@yannbf thank you for feedback. We want to update storybook but we have some problem about it and my teammate created an issue on storybook. Our story file config is like that. I do not import @testing-library/react in our story files.

import {userEvent, within} from '@storybook/testing-library';

play: async ({canvasElement}) => {
    await userEvent.click(within(canvasElement).getByRole('button', {name: /save/i}));
}

@reinertisa
Copy link
Author

@yannbf I talked to Chromatic team and they shared me reference issue #19758 about play function does not work in stories built for Production and when I read this issue a couple of reasons trigger this problem and for us, we changed our build commands and it worked and I will summarize it here for people who will have this problem in the future.

  1. using NODE_ENV=development yarn build-storybook - We tried this and it worked.(Chromatic team shared it with us)
  2. use the latest storybook and matches all libraries version - We did not get latest one yet. Maybe this can be other solution.
  3. testing-library/react and @storybook/testing-library create conflict but we do not import both library in our stories. I ignore it. - This is your solution but we do not import them together so if someone do it, they can solve this conflict.

As a result, today we got our first successful chromatic build and play function trigger RHF(react-hook-form) validations without sleep fn.

Thank you for helping to fix this issue. @shilman @tmeasday @IanVS @bluebill1049 @yannbf

@reinertisa
Copy link
Author

This solution is kind of hack so I will keep this issue open until we get latest storybook.
NODE_ENV=development yarn build-storybook - We tried this and it worked.(Chromatic team shared it with us)

@IanVS
Copy link
Member

IanVS commented Sep 19, 2023

Personally, I found that updating to the latest @storybook/testing-library (which uses user-event@14) solved it for us.

@yannbf
Copy link
Member

yannbf commented Sep 22, 2023

Thank you so much @reinertisa for trying all this out and providing information here!
Given that the original error seems to be solved, I will be closing this issue. If you'd like you can write a new one discussing the act issue, which seems unrelated to React 18. Thank you!

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

No branches or pull requests