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

Support Suspense #1917

Closed
2 of 13 tasks
maclockard opened this issue Nov 27, 2018 · 40 comments · Fixed by #1975
Closed
2 of 13 tasks

Support Suspense #1917

maclockard opened this issue Nov 27, 2018 · 40 comments · Fixed by #1975

Comments

@maclockard
Copy link

maclockard commented Nov 27, 2018

Current behavior

I'm currently getting the following error using a Suspense component in my tests.

Enzyme Internal Error: unknown node with tag 13

Looking at the current react work tags, this is for the suspense component: https://github.com/facebook/react/blob/v16.6.0/packages/shared/ReactWorkTags.js#L44

Looking at the current implementation of detectFiberTag it doesn't look for Suspense components: https://github.com/airbnb/enzyme/blob/enzyme%403.7.0/packages/enzyme-adapter-react-16/src/detectFiberTags.js#L42

I believe the exception is coming here: https://github.com/airbnb/enzyme/blob/enzyme%403.7.0/packages/enzyme-adapter-react-16/src/ReactSixteenAdapter.js#L103 after it hits the default for the switch.

Expected behavior

Can handle suspense components

API

  • shallow
  • mount
  • render

Version

library version
enzyme 3.7.0
react 16.6.3
react-dom 16.6.3
react-test-renderer 16.6.3
adapter (below) 1.7.0

Adapter

  • enzyme-adapter-react-16
  • enzyme-adapter-react-16.3
  • enzyme-adapter-react-16.2
  • enzyme-adapter-react-16.1
  • enzyme-adapter-react-15
  • enzyme-adapter-react-15.4
  • enzyme-adapter-react-14
  • enzyme-adapter-react-13
  • enzyme-adapter-react-helper
  • others ( )
@ljharb ljharb added this to Needs Triage in React 16 via automation Nov 27, 2018
@ljharb
Copy link
Member

ljharb commented Nov 27, 2018

Indeed, Suspense is not yet supported.

@missbruni
Copy link

missbruni commented Nov 29, 2018

Hey, any idea when Suspense will be supported ?

@rodoabad
Copy link

@maclockard are you also using React.lazy?

@maclockard
Copy link
Author

No, currently just Suspense. Will probably start using lazy as well soon though, but its lower priority to me.

@KrisB1022
Copy link

Any update on this? I'm using Suspense with lazy. Just converted over to using, and getting this error. Wasn't sure it there was something being looked at. Thanks in advance.

@anupammaurya
Copy link

no luck found.. on this simulate event handling....

@chenesan
Copy link
Contributor

chenesan commented Jan 7, 2019

Hello @ljharb I'd like to work on this to make React.Suspense and React.lazy work. Will look into this in the next few days. Since these featrues only works after 16.6, we also have to copy current enzyme-adapter-react-16 as enzyme-adapter-react-16.5 package right?

@ljharb
Copy link
Member

ljharb commented Jan 7, 2019

@chenesan for now, just implement it in the 16 adapter; we can iterate in that in the PR.

@chenesan
Copy link
Contributor

For the problem (2) in the PR #1975 comment , after some investigation I think we can just traverse the fiber tree to find all the LazyComponent fiber nodes. Every LazyComponent fiber node hold a elementType object, We can get the promise of dynamic import from elementType._result.

And in the ReactWrapper, I think we can have a new api for waiting lazy loading: updateUntilAllLazyComponentsLoaded(), the implementation may be like:

// helper function to find all lazy components under a fiber node

function findAllLazyNodes(fiberNode) {
  // will return array of LazyComponent fiber node
}

// inside mountRenderer returned by adapter

waitUntilAllLazyComponentsLoaded() {
  const lazyNodes = findAllLazyNodes(instance._reactInternalFiber)
  // the `_result` might be a promise(when pending), an error(when rejected), module(when resolved)
  // will handle all cases in implementaion, now assume there are all promises
  const promises = lazyNodes.map(node => node.elementType._result)
  await Promise.all(promises)
}

// inside ReactWrapper class
updateUntilAllLazyComponentsLoaded() {
  // check if adapter support this
  if (typeof this[RENDERER].waitUntilAllLazyComponentsLoaded() !== "function") {
    throw Error("Current adapter not support React.lazy")
  }
  // wait for lazy load, will handle rejected case in adapter
  await this[RENDERER].waitUntilAllLazyComponentsLoaded()
  // since all of lazy component loaded, force it update to rerender 
  this.update()
}

So we can test the rendering after React.lazy loaded:

const LazyComponent = lazy(() => import('/path/to/dynamic/component'));
const Fallback = () => <div />;
const SuspenseComponent = () => (
    <Suspense fallback={<Fallback />}>
      <LazyComponent />
    </Suspense>
);

const wrapper = mount(<SuspenseComponent />)
expect(wrapper.find('Fallback')).to.have.lengthOf(1)
expect(wrapper.find('DynamicComponent')).to.have.lengthOf(0)

await wrapper.waitUntilLazyLoaded()

expect(wrapper.find('Fallback')).to.have.lengthOf(0)
expect(wrapper.find('DynamicComponent')).to.have.lengthOf(1)

How do you think about the api @ljharb ? Or anyone could comment on this?

@ljharb
Copy link
Member

ljharb commented Jan 13, 2019

I think that since Suspense and lazy don’t actually work yet for their intended purpose (bundle splitting) that we don’t have to rush to invent a new API for it.

@tarjei
Copy link

tarjei commented Jan 13, 2019

@ljharb do you have any info on why lazy doesn't work for code splitting? (I.e. issues etc). I know that Suspense isn't finished, but I thought lazy() worked.

Anyhow, I would love a release that just supported rendering the main path of the api so that using enzyme with react 16.6 could work - even though it didn't support testing all variants of Suspense. Maybe that should be the first target?

@ljharb
Copy link
Member

ljharb commented Jan 13, 2019

Indeed, that’s the first target :-)

@chenesan
Copy link
Contributor

@ljharb Using lazy() to do dynamic import should work. I think this would be useful for people want to test the rendering after dynamic module loaded. I'm sure it's doable to have an api for this, but I'm not sure how many people really need it. If it's not urgent then I can just add the Suspense and Lazy tag into enzyme to make sure the test in React^16.6 will work.

@chenesan
Copy link
Contributor

@ljharb Maybe I misunderstood the "rendering the main path of the api" in @tarjei 's comment. Do you mean that for now we should just support rendering the direct result of lazy() in initial mount (which will be the fallback passed to Suspense)?

@tarjei
Copy link

tarjei commented Jan 14, 2019

@chenesan, @ljharb

I believe I have misunderstood what is supported in 16.6 :) What I tried to say was that it is quite painful to wait for enzyme to support lazy() and the other 16.6 functionality and I was hoping that could be prioritized before supporting all the general parts of the new Suspense component - i.e. the other usages than lazy loading. If it makes it easier I would think a first version that does not support fallback testing would also help.

That said, I'm very much a 👍 on waitUntilLazyLoaded() as that is something I have had to hack on to react-loadable.

Regards, and sorry for creating confusion.
Tarjei

@rodoabad
Copy link

rodoabad commented Jan 14, 2019

@tarjei React.lazy only supports default exports for now. So if you're exporting named modules, those are not going to work unless you are also exporting them as default exports.

It also does not yet support server side rendering.

@chenesan
Copy link
Contributor

chenesan commented Jan 14, 2019

Thanks for your reply @tarjei !
I think there are two parts of work:

(1) make enzyme recognize the lazy and Suspense tag so it won't throw internal error unknown node with tag 13 and we can test the initial mount (which will render fallback for all lazy component under Suspense)

(2) support waiting lazy loading so we are able to test rendering after lazy loading done (like waitForLazyLoaded in previous comment).

I've worked on (1) in #1975 (need some polish and tests). After the discussion I might not do (2) for now since it looks not urgent. If someone needs this comment is welcome ;)

@gaearon
Copy link
Contributor

gaearon commented Jan 18, 2019

The notion of "waiting" for modules in a test environment doesn't quite make sense to me. So I added support for sync resolving of lazy() in facebook/react#14626. In next version, if you give it a { then(cb) { cb(module) } } then it shouldn't suspend at all.

@ljharb ljharb moved this from Needs Triage to v16.6: memo, lazy, suspense in React 16 Jan 23, 2019
@anushshukla
Copy link

any update for a release (be it a major / minor release but stable one) date (tentative one would also do) for the fix of this issue.

@ljharb
Copy link
Member

ljharb commented Feb 6, 2019

@anushshukla no, see #1975.

@VincentLanglet
Copy link

@anushshukla

You can create a mock: __mocks__/react.js

import React from 'react';

const react = jest.requireActual('react');

const Suspense = ({ children }) => <div>{children}</div>;
Suspense.displayName = 'Suspense';

module.exports = { ...react, Suspense };

@thefivetoes
Copy link

thefivetoes commented Feb 19, 2019

EDIT: This mock works for shallow renders, and does not work on mount.

FWIW, here is an updated version of @VincentLanglet's mock that also mocks React.lazy

import React from "react";

const react = jest.requireActual("react");

const Suspense = ({ children }) => <div>{children}</div>;
Suspense.displayName = "Suspense";

const lazy = React.lazy(() =>
  Promise.resolve().then(() => ({
    default() {
      return null;
    }
  }))
);
lazy.displayName = "lazy";

module.exports = { ...react, Suspense, lazy };

@VincentLanglet
Copy link

@thefivetoes When I use you lazy mock, I have no error with shallow but with mount, I get

Error: A React component suspended while rendering, but no fallback UI was specified.

Add a <Suspense fallback=...> component higher in the tree to provide a loading indicator or placeholder to display.

Does it work for you ? This error seems to be normal since I mock the Suspense component...

@thefivetoes
Copy link

@VincentLanglet you are correct, my bad – I am only doing shallow renders.

@anushshukla
Copy link

anushshukla commented Feb 24, 2019

just wanted to thank you guys, @VincentLanglet and @thefivetoes, in writing, for the help.

Also wanted to share that in test cases of components who have nested components using Suspense + Lazy, you can do the following

  1. create mocks in the same folders having such components
  2. create index.js in the mocks folder in the step.1
  3. simply just mock such components in the test cases of components who use have such nested components
    (ref: https://stackoverflow.com/questions/44403165/using-jest-to-mock-a-react-component-with-props).

In my case,

AsyncComponent/index.js which uses Suspense + Lazy

AsyncComponent/mocks/index.js which is an alternate to Suspense + Lazy

In the failing test file, just added the below line,

jest.mock('path/to/AsyncComponent');

Also, we are utilising React test renderer other than enzyme and would suggest the same to others that not to rely on one testing utility while we all wish to have that ideal one but reality is this.

@alex-hall
Copy link

alex-hall commented Mar 19, 2019

@thefivetoes did you do anything special to get that mock off the ground?

No matter what I do, I end up with the following exception:

    RangeError: Maximum call stack size exceeded
    > 1 | import React from "react";
        | ^
      2 | 
      3 | const react = jest.requireActual("react");
      4 | 

I tried with a custom webpack + babel config as well as ejecting from the latest create-react-app (plus bumping the react + enzyme versions).

EDIT:

Removing the initial import allows the mock to actually fire, but the mocked lazy implementation doesn't seem to match the actual lazy implementation.

@AnatoliyLitinskiy

This comment has been minimized.

@ljharb

This comment has been minimized.

@ljharb
Copy link
Member

ljharb commented Jun 4, 2019

v3.10.0 has now been released.

@shridharkalagi
Copy link

@thefivetoes How to make this work on mount? Can you please help?

@rodoabad
Copy link

Anyone got React.lazy to work without affecting coverage?

Screen Shot 2019-08-14 at 11 49 32 AM

While the test is able to assert that the component is being loaded, jest is reporting that the imported component is not being covered.

@rodoabad
Copy link

Not sure if @ljharb (Jordan) is still getting replies on this thread so I'll tag him directly (sorry, Jordan if you already are!) ❤️

@ljharb
Copy link
Member

ljharb commented Aug 31, 2019

@rodoabad i am, but haven't had time to get to all of my hundreds of notifications yet. please file a new issue, since code coverage of React.lazy isn't the same as overall Suspense support.

@shridharkalagi similarly, if you're still having an issue, please file a new issue.

@Hypnosphi
Copy link

Quoting #1917 (comment)

The notion of "waiting" for modules in a test environment doesn't quite make sense to me. So I added support for sync resolving of lazy() in facebook/react#14626. In next version, if you give it a { then(cb) { cb(module) } } then it shouldn't suspend at all.

@ljharb given that, do you think that https://github.com/airbnb/babel-plugin-dynamic-import-node could support sync thenable mode? That is, transforming import('foo') to { then(cb) { cb(require('foo')) } }

@ljharb
Copy link
Member

ljharb commented May 20, 2020

No, I don't - that would violate the spec (altho an older version of that transform did operate that way; you could always peg to that version).

Either way, forcing a babel transform so that enzyme works properly both won't work in browsers and doesn't seem ideal.

@Hypnosphi
Copy link

OK I see, I just wonder how can I actually benefit from facebook/react#14626

@tmkasun
Copy link

tmkasun commented May 20, 2020

I tried with 3.11.0 but getting an error when using mount with a lazy-loaded component in the tree

@sirshurak
Copy link

sirshurak commented Nov 12, 2020

For coverage propose, I've done the following code

jest.mock('react', () => { const React = jest.requireActual('react'); return { ...React, Suspense: ({ children }) => <div>{children}</div>, lazy: (factory) => factory() } });

@ShailyAggarwalK
Copy link

ShailyAggarwalK commented Dec 9, 2021

I needed to test my lazy component using Enzyme. Following approach worked for me to test on component loading completion:

const myComponent = React.lazy(() => 
          import('@material-ui/icons')
          .then(module => ({ 
             default: module.KeyboardArrowRight 
          })
       )
    );

Test Code ->

//mock actual component inside suspense
 jest.mock("@material-ui/icons", () => { 
     return {
         KeyboardArrowRight: () => "KeyboardArrowRight",
 }
 });
 
 const lazyComponent = mount(<Suspense fallback={<div>Loading...</div>}>
            {<myComponent>}
        </Suspense>);
     
 const componentToTestLoaded  = await componentToTest.type._result; // to get actual component in suspense
     
 expect(componentToTestLoaded.text())`.toEqual("KeyboardArrowRight");

This is hacky but working well for Enzyme library.

@FirstWhack
Copy link

FirstWhack commented Jan 5, 2022

@ShailyAggarwalK I'm not sure what this example means seeing as React.lazy does not return something with a type property as shown in your example.. Assuming componentToTest is supposed to be myComponent (not defined in your example) - https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/react/index.d.ts#L872

The result of running this code in any normal environment would be something like cannot read property _result of undefined

_result couldn't be a Promise here either if it wasn't already resolved (and your resolved value was a Promise, in which case you're not awaiting lazy but the result of it... ).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
React 16
  
v16.6+: memo, lazy, suspense
Development

Successfully merging a pull request may close this issue.