Support React.lazy and React.Suspense #1975

chenesan · 2019-01-10T17:11:09Z

Fixes #1917 .

Given a component wrapped by React.lazy in <Suspense />
It'll plainly render a <Lazy /> in shallow and render fallback component in mount.

There are something I'm not sure / still working on:

What should displayName of the component returned by React.lazy be? Currently I directly named it as Lazy. Not sure if it's something we could define by ourselves.
I'm trying to add an waitUntilLazyLoaded() on ReactWrapper, which will return a promise resolving when the dynamic import loaded and React trigger the re-render, so we can write some tests like:

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

const wrapper = mount(<SuspenseComponent />)
await wrapper.waitUntilLazyLoaded()

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

But I don't know how to detect if all the lazy loaded component inside <Suspense /> has completeted loading. It looks like that we have to hack around react fiber. @ljharb Would you know any way to detect this?

Also note that this PR add babel-plugin-dynamic-import-node and babel-plugin-syntax-dynamic-import for using import(), babel-eslint in enzyme-adapter-react-16 and enzyme-test-suite for dynamic import support of eslint.

rodoabad · 2019-01-13T18:39:40Z

@chenesan are you not able to determine the display name of the component that is being loaded by React.lazy? Once the promise has resolved you should have full access to the lazy loaded component correct?

chenesan · 2019-01-14T00:11:38Z

@rodoabad I'm not sure what the displayName of the wrapper component returned by React.lazy is. See the test in https://github.com/airbnb/enzyme/blob/master/packages/enzyme-test-suite/test/Adapter-spec.jsx#L987-L1011 and https://github.com/airbnb/enzyme/blob/master/packages/enzyme-adapter-react-16/src/ReactSixteenAdapter.js#L493-L525

chenesan · 2019-01-15T10:02:20Z

@ljharb I just force pushed the branch and I think it's ok to review it now.

Basically this PR will supports shallow/mount with Suspense and React.lazy.

ljharb

Thanks, this is a great start.

ljharb · 2019-01-16T06:26:10Z

.babelrc

@@ -2,6 +2,8 @@
  "presets": ["airbnb"],
  "plugins": [
    ["transform-replace-object-assign", { "moduleSpecifier": "object.assign" }],
+    "syntax-dynamic-import",
+    "dynamic-import-node"


let's only add these in the "test" env; i don't think this is ever a safe transform except at the app level.

ljharb · 2019-01-16T06:26:41Z

packages/enzyme-adapter-react-16/.eslintrc

@@ -1,5 +1,6 @@
 {
  "extends": "airbnb",
+  "parser": "babel-eslint",


this is also not acceptable; babel-eslint enables all syntax currently, so airbnb projects only use the default parser.

Suggested change

"parser": "babel-eslint",

ljharb · 2019-01-16T06:30:19Z

packages/enzyme-adapter-react-16/src/detectFiberTags.jsx

@@ -38,6 +67,9 @@ module.exports = function detectFiberTags() {
    // eslint-disable-next-line no-unused-vars
    FwdRef = React.forwardRef((props, ref) => null);
  }
+  if (supportsLazy) {
+    LazyComponent = React.lazy(() => import('./_helpers/dynamicImportedComponent'));


There's zero point in using import() here - it has to be transpiled, and it's never safe to transpile it below the app level, so this would have to go untranspiled, and that'd be a breaking change.

It'd be much simpler to use () => Promise.resolve().then(() => ({ default: require(…) })).

Separately, I'm not sure why this can't be even simpler - React.lazy(() => Promise.resolve().then(() => ({ default() { return null; } }))

Ah... yeah, the Promise.resolve().then(() => ({ default() { return null; } })) indeed works. If so I think all the added eslint plugins for import syntax, fake module file and babel-eslint can be removed. Thanks for this solution!

For what it's worth, this way is much cleaner in the sense that it preserves a single source of truth of "how import() should work in node" - but due to the underspecified nature of ESM, until node ships unflagged ESM, it's not safe for anything but the top-level app to define "how import() works", unfortunately.

I think maybe it's worth to add a utility to make it clearer like

const fakeDynamicImport = (module) => Promise.resolve().then(() => ({ default: module})) // so we can write const LazyComponent = React.lazy(() => fakeDynamicImport(SomeComponent))

for all the places using React.lazy in enzyme packages. But I'm not sure where to put this utility.

Sounds great! If it'd be used by only adapters, it should be in adapter-utils; if by enzyme as well, then probably in enzyme itself; since it's likely to only be used in the 16 adapter for now but the 17 adapter later, probably in adapter-utils?

React.lazy only appears in 16 adapter and tests. Looks like adapter-utils is a good place.

ljharb · 2019-01-16T06:31:27Z

packages/enzyme-test-suite/.eslintrc

@@ -1,6 +1,7 @@
 {
  "extends": "airbnb",
  "root": true,
+  "parser": "babel-eslint",


Suggested change

"parser": "babel-eslint",

ljharb · 2019-01-16T06:31:50Z

packages/enzyme-test-suite/package.json

    "object-inspect": "^1.6.0",
+    "object.assign": "^4.1.0",


not sure why this got re-sorted, what version of npm did you use to install it?

It looks like I accidentally add new package with yarn. will fix this.

ljharb · 2019-01-16T06:34:36Z

packages/enzyme-test-suite/test/_helpers/dynamicImportedComponent.jsx

@@ -0,0 +1,5 @@
+import React from 'react';
+
+const DynamicComponent = () => <div>Dynamic Component</div>;


this entire file seems unnecessary if it exists in enzyme

ljharb · 2019-01-16T06:35:25Z

packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx

+
+        const wrapper = shallow(<SuspenseComponent />);
+
+        expect(wrapper.find('Suspense')).to.have.lengthOf(1);


Since a shallow wrapper is "what the wrapped thing renders", i would expect this to be:

Suggested change

expect(wrapper.find('Suspense')).to.have.lengthOf(1);

expect(wrapper.is(Suspense)).to.equal(true);

ljharb · 2019-01-16T06:36:06Z

packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx

+
+        expect(wrapper.find('Suspense')).to.have.lengthOf(1);
+        expect(wrapper.find(LazyComponent)).to.have.lengthOf(1);
+        // won't render fallback in shallow renderer


can we add another test that .dive()s the Suspense, and then becomes the fallback?

Oops, I forgot to test shallow render Suspense directly, and I found out that for now react-test-renderer/shallow doesn't allow render Suspese:

import ShallowRenderer from "react-test-renderer/shallow" const renderer = new ShallowRenderer() renderer.render(<Suspense fallback={false} />)

will throw:

Invariant Violation: ReactShallowRenderer render(): Shallow rendering works only with custom components, but the provided element type was `symbol`. at invariant (/Users/yishan/Documents/Projects/enzyme/packages/enzyme-adapter-react-16/node_modules/react-test-renderer/cjs/react-test-renderer-shallow.development.js:54:15) at ReactShallowRenderer.render (/Users/yishan/Documents/Projects/enzyme/packages/enzyme-adapter-react-16/node_modules/react-test-renderer/cjs/react-test-renderer-shallow.development.js:382:78)

It looks like that react shallow renderer cannot recoginze the Suspense type symbol. I should check this at first... So unless react-test-renderer supports this I think we can only support Suspense and lazy in mount. 😢

ljharb · 2019-01-16T06:36:16Z

packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx

+
+        const wrapper = shallow(<SuspenseComponent />);
+
+        expect(wrapper.find('Suspense')).to.have.lengthOf(1);


Since a shallow wrapper is "what the wrapped thing renders", i would expect this to be:

Suggested change

expect(wrapper.find('Suspense')).to.have.lengthOf(1);

expect(wrapper.is(Suspense)).to.equal(true);

ljharb · 2019-01-16T06:36:36Z

packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx

+          }
+        }
+        const SuspenseComponent = () => (
+          <Suspense fallback={false}>


also here, let's pass a Fallback

chenesan · 2019-01-16T09:13:30Z

Fixed problems except the .dive() one, which makes me find out that react-test-renderer/shallow not support render Suspense. Feel sorry about not checking this earlier :(

chenesan · 2019-01-17T23:52:05Z

@ljharb I'm wondering if I should work on shallow renderer in react side to make progress for this PR.
If so, I'm not sure how shallow renderer should handle fallback of Suspense. Should we just return the children, or traverse the children of Suspense to find all lazy component and transform this to fallback element? I know this seems a bit off topic here, though.

chenesan · 2019-01-19T13:31:26Z

Hi @ljharb I tried to start a PR in facebook/react#14638 to support shallow rendering Suspense in react shallow renderer. I'm not sure if I did it right for enzyme. I guess you may be interested in looking into this ;)

chenesan · 2019-01-21T14:38:58Z

I'm not sure when react-test-renderer/shallow will support Suspense and it looks like it'd need a long time(has started a PR in facebook/react#14638 and waiting for reply). So I think we can just concentrate on supporting Suspense and lazy in mount in this PR to make it work sooner. Once shallow renderer support this we can create a new PR. I just removed the shallow wrapper test to prevent misleading.

@ljharb I think we can start a new round of review. If you think it's better to wait for shallow renderer support, let me know and I'll add tests back, thanks :)

ljharb

This looks good, but I think we should re-add the shallow tests.

If the shallow renderer won't support it, then we'll have to support it for them.

chenesan · 2019-01-22T07:51:30Z

ok, I'll add them back. If there's still no reply in react this week I'll start to work on support Suspense in adapter 16.

And if we have to support Suspense in enzyme side, how should we handle the fallback inside Suspense? I think we should keep behavior of shallow as same as mount, and we could just turn unresolved lazy component node in children to fallback node. But if we keep .dive() down into the children and get a lazy component node, should we handle fallback for it? Or we should just throw error? I'd prefer to the latter since React doesn't support render lazy component independently, but I'm not sure if this behavior would confuse user.

ljharb · 2019-01-22T22:48:22Z

@chenesan it kind of depends on what react's planning to do.

In other words, in general, we want shallow and mount to be as close as possible. However, when you're shallow rendering something that renders a Suspense or a lazy, those things should just show up in the tree - when you dive through one of them, or when you shallow render one of them directly, then their implementation should be "invoked", whatever that ends up meaning.

Similarly, it should be possible (in both shallow and mount) to force a Suspense to render the lazy component or the fallback, so both paths can be tested.

chenesan · 2019-01-22T23:53:53Z

So when we shallow render Suspense

const LazyComponent = React.lazy(() => import('InternalComponent'))
shallow(<Suspense fallback={<Fallback />}>
  <LazyComponent />
</Suspense>)

then should it render a fallback, or the <LazyComponent /> -- but not invoke the lazy load function (which should be invoked when we shallow render <LazyComponent />?

And another problem is when we shallow(<LazyComponent />), there seems to be some possible behaviors:

throw error, since React not allow singly render lazy component
wait it load until it resolved or error, if error we have to give it a fallback. But it's tricky to wait for loading in tests.
add option in shallow, or we could provide some utility resetLoader to replace LazyComponent loader function in run time, to replace LazyComponent with implementation when shallow rendering.

I think 1+3 sounds reasonable.

Similarly, it should be possible (in both shallow and mount) to force a Suspense to render the lazy component or the fallback, so both paths can be tested.

it sounds like we can pass an option when shallow rendering to determine how to render Suspense - maybe a "normal" mode to render them as same as what React does and a "fallback" mode to render them all as fallback.

ljharb · 2019-01-23T07:19:00Z

then should it render a fallback, or the -- but not invoke the lazy load function (which should be invoked when we shallow render ?

Yes. Whether it should render the fallback or the LaxyComponent, I have no idea - (note that this only comes up when the Provider is the component being rendered).

As for LazyComponent, that's up to the author of the test - it seems like perhaps we only need a mechanism for the developer to instruct enzyme whether to use the fallback or to use the child.

chenesan · 2019-01-23T09:31:30Z

ok, so I'll add an option (maybe suspenseFallback: boolean) into shallow and pass it down to adapter. When shallow(<Suspense fallback={<Fallback />}>/* children */</Suspense>), if it's true, it will traverse through the children and replace all of <LazyComponent /> with <Fallback />, else we will keep them there.
And for shallow(<LazyComponent />) just let shallow renderer throw error(maybe rethrow with an clearer error message).
I'll work on them after this week if there's still no feedback from react side. Thanks for discussion :)

ljharb · 2019-01-23T22:34:22Z

Sounds great. The only concern I think that's left is "what happens when you shallow render a lazy component", and it'd be great to know what React plans in the shallow renderer before we implement it here (we don't have to wait on them releasing that, we just need to know what they'll do)

chenesan · 2019-04-16T23:58:11Z

@ljharb Still not figured this out :-( Sometimes the coverage recovered after I removed some lines of code, but when I tried to reproduce again it still dropped. Wondering if it's an issue in istanbul side -- Do you have any idea around this?

chenesan · 2019-04-21T06:44:58Z

@ljharb Get this done in e60788c. It seems in some places we import fakeDynamicImport from enzyme-adapter-utils/src/Utils rather than enzyme-adapter-utils, which cause nyc count coverage incorrectly.

chenesan · 2019-04-23T07:02:25Z

@ljharb Sorry for pinging again :-| Just fixed the coverage issue in e60788c so I think this could be reviewed / merged now?

DonikaV · 2019-05-02T13:37:30Z

Sorry, i have a question. When i can use enzyme, shallow with lazy and Suspense? Is there any updates of library in the future? Merge was 8 days ago. But i see that the version of enzyme is the same...
Thanks in advance.

ljharb · 2019-05-02T15:40:32Z

This will be in the next release; there’s no timeline for it.

- [new] add `fakeDynamicImport` (#1975) - [deps] update `airbnb-prop-types` - [dev deps] update `eslint-plugin-react`, `eslint-plugin-import`

- [new] support `suspenseFallback` option; support `Suspense`/`Lazy` (#1975) - [fix] `shallow`: properly dive through `memo` components (#2103, #2068) - [fix] avoid a Context.Provider deprecation warning - [fix] shallow renderer for `memo` does not respect `defaultProps` (#2115) - [fix] Don’t show wrapped component displayName in lazy component (#1975) - [fix] `simulateEvent`: call the adapter’s implementation, not the raw one (#2100) - [deps] update `enzyme-adapter-utils` - [dev deps] update `eslint-plugin-react`, `eslint-plugin-import`

GitHub-Mar · 2019-05-14T07:15:09Z

I noticed this was released 3 days ago in v 1.13 of enzyme-adapter-react-16. Does Enzyme now fully support dynamically imported components and wrapping? We are still struggling to get this working for our local tests.

Is there any documentation or working examples someone could point me to? I've been trying to test a dynamically imported component that is not wrapped in a Suspense component as we are using a Suspense higher up the component tree to wrap everything, however, I'm unsure how to tell enzyme to resolve the promise and load through the component? If I debug I can see the node as a child of my mounted wrapper but it's just listed as a react.lazy.

I believe this is the correct strategy when using Suspense components, though I'm also quite new to this and may well be wrong!

ljharb · 2019-05-14T18:50:11Z

@GitHub-Mar no, this pr has two components, enzyme itself (not released) and the adapter (released).

GitHub-Mar · 2019-05-15T07:08:29Z

Ah my mistake! So this feature is not currently supported in Enzyme, then? I'm guessing your above comment still stands and there is no timeline as to when this will be released?

ljharb · 2019-05-15T12:44:24Z

Correct.

ljharb · 2019-06-04T00:58:09Z

v3.10.0 has now been released.

DonikaV · 2019-06-07T10:28:36Z

Still failing. Do you have some docs for this? I mean maybe i should update my tests or something like this?
Thanks

ljharb · 2019-06-07T20:12:56Z

@DonikaV all the docs are published. If you've fully updated enzyme and your adapter, please file a new issue and fill out the template.

shridharkalagi · 2019-08-06T10:17:08Z

Can I use Enzyme mount API with Lazy and Suspense yet?

ron23 · 2020-12-11T19:21:52Z

I'm still a bit confused on what's the best way to test that a component wrapped in lazy renders inside Suspense.
Say I have:

function myComponent = (props) => {
   const MyLazy = React.lazy(() => import('./MyLazyComponent');
   return <Suspense fallback={...}><MyLazy/></Suspense>
}

If I use the method mentioned above to mock react:

jest.mock('react', () => {
  const React = jest.requireActual('react');
  React.Suspense = ({ children }) => children
  return React;
});

The debug() returns something like:

<Component fallback={{...}}>
      <lazy />
    </Component>

I want to test that MyComponent actually renders MyLazyComponent.
I tried mount and shallow and neither works.

enzyme@3.11.0
enzyme-adapter-react-16@1.15.2

[enzyme-adapter-utils] [new] add fakeDynamicImport

4d2bad1

This was referenced Jan 12, 2019

Question: How to know if the loading status of Suspense resolved facebook/react#14577

Closed

Support Suspense #1917

Closed

chenesan force-pushed the react-suspense branch from e656f7a to 3577a02 Compare January 15, 2019 09:59

chenesan changed the title ~~[WIP] Support React.lazy and React.Suspense~~ Support React.lazy and React.Suspense Jan 15, 2019

ljharb requested changes Jan 16, 2019

View reviewed changes

ljharb added API: shallow API: mount semver: minor New stuff. package: react adapter: 16 labels Jan 16, 2019

chenesan force-pushed the react-suspense branch from 1652d74 to 1bbe447 Compare January 17, 2019 10:05

This was referenced Jan 18, 2019

react-test-renderer doesn't support Suspense and lazy nodes facebook/react#14170

Open

Support Suspense in shallow renderer facebook/react#14638

Closed

chenesan force-pushed the react-suspense branch from 1bbe447 to 7458330 Compare January 21, 2019 14:28

ljharb reviewed Jan 22, 2019

View reviewed changes

chenesan force-pushed the react-suspense branch from 7458330 to 8a1dde1 Compare January 23, 2019 03:14

ljharb force-pushed the master branch 2 times, most recently from 40cc703 to 0a17404 Compare April 12, 2019 23:05

prymitive mentioned this pull request Apr 16, 2019

feat(ui): lazy load modals prymitive/karma#631

Merged

chenesan force-pushed the react-suspense branch from 080187c to 3139d43 Compare April 16, 2019 23:54

ljharb force-pushed the react-suspense branch from e60788c to b5afdb9 Compare April 24, 2019 07:20

ljharb approved these changes Apr 24, 2019

View reviewed changes

ljharb merged commit b5afdb9 into enzymejs:master Apr 24, 2019

ljharb added a commit that referenced this pull request May 7, 2019

[enzyme-adapter-utils] v1.12.0

e3fb6b3

- [new] add `fakeDynamicImport` (#1975) - [deps] update `airbnb-prop-types` - [dev deps] update `eslint-plugin-react`, `eslint-plugin-import`

json2d mentioned this pull request Jul 30, 2019

mount Suspense and throw promise - Enzyme Internal Error: unknown node with tag 2 #2205

Open

13 tasks

snyk-bot mentioned this pull request Oct 3, 2019

[Snyk] Upgrade enzyme-adapter-react-16 from 1.3.1 to 1.14.0 bermudez/nkc-webapp#25

Open

		@@ -0,0 +1,5 @@
		import React from 'react';

		const DynamicComponent = () => <div>Dynamic Component</div>;


		const wrapper = shallow(<SuspenseComponent />);

		expect(wrapper.find('Suspense')).to.have.lengthOf(1);

	expect(wrapper.find('Suspense')).to.have.lengthOf(1);
	expect(wrapper.is(Suspense)).to.equal(true);

Support React.lazy and React.Suspense #1975

Support React.lazy and React.Suspense #1975

Conversation

chenesan commented Jan 10, 2019 • edited by ljharb Loading

rodoabad commented Jan 13, 2019

chenesan commented Jan 14, 2019

chenesan commented Jan 15, 2019

ljharb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chenesan Jan 16, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chenesan Jan 16, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chenesan commented Jan 16, 2019

chenesan commented Jan 17, 2019

chenesan commented Jan 19, 2019

chenesan commented Jan 21, 2019

ljharb left a comment

Choose a reason for hiding this comment

chenesan commented Jan 22, 2019

ljharb commented Jan 22, 2019 • edited Loading

chenesan commented Jan 22, 2019

ljharb commented Jan 23, 2019

chenesan commented Jan 23, 2019

ljharb commented Jan 23, 2019

chenesan commented Apr 16, 2019

chenesan commented Apr 21, 2019

chenesan commented Apr 23, 2019

DonikaV commented May 2, 2019

ljharb commented May 2, 2019

GitHub-Mar commented May 14, 2019

ljharb commented May 14, 2019

GitHub-Mar commented May 15, 2019

ljharb commented May 15, 2019

ljharb commented Jun 4, 2019

DonikaV commented Jun 7, 2019

ljharb commented Jun 7, 2019

shridharkalagi commented Aug 6, 2019

ron23 commented Dec 11, 2020 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

chenesan commented Jan 10, 2019 •

edited by ljharb

Loading

chenesan Jan 16, 2019 •

edited

Loading

chenesan Jan 16, 2019 •

edited

Loading

ljharb commented Jan 22, 2019 •

edited

Loading

ron23 commented Dec 11, 2020 •

edited

Loading