jest-react-fela | jest-fela-bindings: Support snapshotting in both jsdom and node environments

[alizeait]

Description

We currently assume node environment only for all snapshots, but that's not the case when running tests in jsdom. This should hopefully support both cases now as well as React 18 and 17

Packages

List all packages that have been changed.

• jest-react-fela
• jest-fela-bindings

Versioning

Minor

Checklist

Quality Assurance

You can also run pnpm run check to run all 4 commands at once.

• The code was formatted using Prettier (pnpm run format)
• The code has no linting errors (pnpm run lint)
• All tests are passing (pnpm run test)

Changes

If one of the following checks doesn't make sense due to the type of PR, just check them.

• Tests have been added/updated
• Documentation has been added/updated

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
fela	✅ Ready (Inspect)	Visit Preview	May 21, 2022 at 2:07PM (UTC)

[robinweser]

The problem is, that this is not going to work with React 18 which is now the stable release. You‘ll get a „render is deprecated“ warning for every snapshot 😕

[alizeait]

I see, how about we either

• Create a second React < 18 export that uses render
• try/catch react 18 like we did previously but then projects on React 18 still using render fail
• or allow passing extra arguments to createSnapshot to decide between render/createRoot, this can be helpful if people are on React 18 and still using render

[robinweser]

I think 2 separate exports in the same package would be my preferred way.
I tried it before, but I couldn't get React 18 with createRoot to output any markup for jest since it renders async now and thus one would have to await the snapshot in every test instead.

TBH I think the current renderToStaticMarkup is all you need for snapshots. I'd even argue that snapshots with jsdom logic in it are out of scope for component snapshot tests.

[alizeait]

The problem now is the usage of useLayoutEffect on the server. The recommended way by the React team is to replace useLayoutEffect with useEffect server-side like:

const useUniversalLayoutEffect =
  typeof window !== 'undefined' ? useLayoutEffect : useEffect;

The problem with using renderToStaticMarkup in a jsdom environment is that the above snippet would return useLayoutEffect since window is defined in jsdom and React will complain about using useLayoutEffect server-side

[alizeait]

@robinweser I reverted the changes to the current sync createSnapshot (though this will still fail with useLayoutEffect). I instead created new async methods that handle Node and JSDOM environments for both React 18 and React 17. We now wait for React to completely finish in both React 18 and 17 before outputting snapshots.
I added tests for Node and React 18 as well

[alizeait]

@robinweser does this look like something that works?

[robinweser]

Jup, looks good to me, thanks! I'll look into it tomorrow and try to do a release asap, but I'm quite busy at the moment so sorry in advance if it takes another day or two.