Skip to content

React.lazy() #64

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.

Sign up for GitHub

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

Merged
gaearon merged 3 commits into from
Oct 23, 2018
Merged

React.lazy() #64

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7c2b878
React.lazy()
gaearon Oct 19, 2018
cf3da2d
Mention future named export support proposal
gaearon Oct 19, 2018
bb55746
Clarify and fix typo
gaearon Oct 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Mention future named export support proposal
  • Loading branch information
@gaearon
gaearon authored Oct 19, 2018
commit cf3da2d484051e77a10498f09251726c4d1a0537
15 changes: 13 additions & 2 deletions text/0000-lazy.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ and not like this:

```js
// Annoying and confusing:
const Button = lazy(() => import('./Button').then(Button => Button.default);
const Button = lazy(() => import('./Button').then(Button => Button.default));
Copy link

@streamich streamich Oct 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does not have to be written like that. Could be

const Button = lazy(async () => (await import('./Button')).default);

or even

const Button = lazyDefault(() => import('./Button'));

or

const Button = lazy(() => import('./Button'), 'default');

or

const Button = lazy(() => import('./Button').then(_ => _.default));

Copy link

@milesj milesj Oct 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It most definitely needs to be a function that returns a promise, so the last 3 examples are invalid.

Copy link

@streamich streamich Oct 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@milesj thx, corrected

Copy link

@gabemeola gabemeola Oct 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it'll need the intermediate function so the dynamic import isn't immediately resolved.

Copy link

@milesj milesj Oct 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, without the function it's a side-effect.

// Named imports don't make this better:
const Button = lazy(() => import('./Button').then(Button => Button.Button);
const Button = lazy(() => import('./Button').then(Button => Button.Button));
Copy link
Contributor

@TrySound TrySound Oct 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is better

const Button = lazy(() => import('./Button').then(module => module.Button));

Copy link
Member Author

@gaearon gaearon Oct 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The motivation for why it's not this way is explained just below. Did you get a chance to read it?

```

(Note this doesn't mean you're forced to use default exports for *all* your components. Even if you primarily use named exports, consider default exports to be "async entry points" into just the components you want to code split.)
Copy link
Contributor

@TrySound TrySound Oct 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds inconsistency in modules style. The point of component code splitting to be invisible. You take any module, wrap it with lazy and it becomes loadable. Converting to default export is additional and wrong action. We don't need to define that module as asynchronous. Module should be only consumed as asynchronous in place.

Copy link
Member Author

@gaearon gaearon Oct 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The motivation for this is explained just below.

Expand All @@ -74,6 +74,17 @@ const Button = lazy(async () => {

We **intentionally** don't support this in the scope of this proposal. This gives React an opportunity to integrate more tightly with the module system on the server in the future. There are no proposed standards for this yet, but the new Suspense-capable React server renderer we'll soon be working on will benefit from having a chance to introspect the `import()` result directly (rather than just a component). For example, it could change a priority of a pending request for the `Button.js` module once it knows that a component of the `Button` type is going to be lazily rendered. We can't do this if we break the link between the module and the component. While this depends on future experimentation and standardization work, this design leaves more space for it. We can always remove this restriction later if it doesn't end up being beneficial. (That's also why the proposal doesn't support arbitrary Promises as component types.)

### Named Exports

While named exports aren't currently supported by this proposal, it doesn't exclude them in the future. For example:

```js
// Not a part of this RFC but plausible in the future
const Button = lazy(() => import('./components'), components => components.Button);
Copy link
Contributor

@TrySound TrySound Oct 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is nice, but we need something right now. Would be good to add at least an ugly example.

const Button = lazy(() =>
  import('./components').then(module => ({ default: module.Button }))
);

Copy link
Collaborator

@sebmarkbage sebmarkbage Oct 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think maybe we’ll try to warn against that pattern since it will break future work

Copy link
Contributor

@TrySound TrySound Oct 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get. How this will break future work? And how are you gonna forbid this? It's just a promise with object. We need a solution now. Using default exports for async components is awful workaround.

Copy link
Member Author

@gaearon gaearon Oct 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you feel so strongly against writing a single line of code that does the export?

Copy link

@theKashey theKashey Oct 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now default exports are discouraged. There are a lot of reasons behind:

  • like allowSyntheticDefaultImports:false in typescript
  • like autoimports in IDE
  • like auto "naming" things

Why not to support basic "babel" interop, ie use .default if _esModules set, mimicking how "real"(babel/webpack) imports work?

I also prefer to have a default export on code splitting point, as some sort of an "interface", but it should not be the law.

Copy link
Member Author

@gaearon gaearon Oct 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Babel interop has made the ESM ecosystem very messy and complex (even though it was instrumental in getting people to adopt ESM — perhaps too early). We don’t wait to add more tooling dependency on this behavior because it’s super hard to fix properly.

I also prefer to have a default export on code splitting point, as some sort of an "interface", but it should not be the law.

See #64 (comment).

```

This is out of scope of the current initial proposal but could be added later.

# Drawbacks

* You can't render a `React.lazy` component with the current server renderer implementation because it doesn't support suspending. (That's the case for all Suspense features so it's not unique to this proposal.)
Expand Down