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

Bug: (17.0.0-rc.1) lazy is eager in dev mode #19748

Closed
nstepien opened this issue Sep 2, 2020 · 8 comments · Fixed by #19871
Closed

Bug: (17.0.0-rc.1) lazy is eager in dev mode #19748

nstepien opened this issue Sep 2, 2020 · 8 comments · Fixed by #19871

Comments

@nstepien
Copy link

nstepien commented Sep 2, 2020

React version: 17.0.0-rc.1

Steps To Reproduce

  1. Create elements out of lazy components, use them conditionally.
  2. Check if the lazy components are dynamically imported.

Link to code example: --

main.jsx:

import React, { lazy, Suspense, useState } from 'react';
import { render } from 'react-dom';

const Checked = lazy(() => import('./Checked'));
const Checked2 = lazy(() => import('./Checked2'));
const Unchecked = lazy(() => import('./Unchecked'));
const Unchecked2 = lazy(() => import('./Unchecked2'));

function App() {
  const [checked, setChecked] = useState(false);

  const checkedElement = <Checked />;
  const uncheckedElement = <Unchecked />;

  return (
    <>
      <label>
        <input
          type="checkbox"
          checked={checked}
          onChange={e => setChecked(e.target.checked)}
        />
        Toggle me
      </label>

      <hr />

      <Suspense fallback="loading...">
        Checked? {checked ? checkedElement : uncheckedElement}
      </Suspense>

      <hr />

      <Suspense fallback="loading...">
        Checked? {checked ? <Checked2 /> : <Unchecked2 />}
      </Suspense>
    </>
  );
}

render(<App />, document.getElementById('app'));

Checked.jsx:

export default function() {
  return 'Checked';
}

Checked2.jsx:

export default function() {
  return 'Checked 2';
}

Unchecked.jsx:

export default function() {
  return 'Unchecked';
}

Unchecked2.jsx:

export default function() {
  return 'Unchecked 2';
}

The current behavior

The Checked.jsx chunk is imported eagerly, even though the component isn't immediately rendered.
image

This doesn't happen in prod mode, or with React 16, or with components that are not createElement-ed eagerly.

This affects react-router, as it uses this pattern:

<Router>
  <Suspense fallback={<Loading />}>
    <Switch>
      <Route exact path="/">
        <Home />
      </Route>
      <Route exact path="/demo">
        <Demo />
      </Route>
      <Route>
        <NotFound />
      </Route>
    </Switch>
  </Suspense>
</Router>

AFAICT it doesn't break anything, but it is unexpected/confusing.

The expected behavior

lazy components that are not rendered should not be dynamically imported.

@nstepien nstepien added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Sep 2, 2020
@nstepien
Copy link
Author

nstepien commented Sep 3, 2020

Shorter test case:

import React, { lazy } from 'react';

const Checked = lazy(() => import('./Checked'));

<Checked />;

This is enough to trigger the dynamic import in dev mode.

@eps1lon
Copy link
Collaborator

eps1lon commented Sep 3, 2020

Behavior changed between 16.13.1 and the first next release after 16.13.1.

Diff between 16.13.1 and 0.0.0-d7382b6c4

@gaearon
Copy link
Collaborator

gaearon commented Sep 3, 2020

This is triggered by getComponentName in validatePropTypes. I think this doesn't really make sense to call but we'd need to see why this change to getComponentName was made. @sebmarkbage?

Screenshot 2020-09-03 at 13 56 14

Screenshot 2020-09-03 at 13 56 18

@sebmarkbage
Copy link
Collaborator

The suspense protocol is just a function that may throw. That allows for low overhead abstractions to be wrapped. This change was to allow the init function to encapsulate the actual suspense mechanism and that data structure.

Unfortunately there’s no way to query the status of something in the suspense protocol without triggering the fetch.

Same as you can’t do:

var data = readData(...);
validateData(data);

Without triggering a fetch.

The solution might be to move validation of lazy until it’s rendered instead of in createElement.

Also, sigh PropTypes.

@nstepien
Copy link
Author

nstepien commented Sep 8, 2020

Should the Unconfirmed label be removed?

@gaearon gaearon added Type: Bug and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Sep 8, 2020
@gaearon
Copy link
Collaborator

gaearon commented Sep 20, 2020

#19871

@gaearon
Copy link
Collaborator

gaearon commented Sep 22, 2020

Should be fixed in 17.0.0-rc.2. Please verify.

@nstepien
Copy link
Author

@gaearon Works great now, thanks!
Looking forward to the 17.0.0 release.

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

Successfully merging a pull request may close this issue.

4 participants