Add support for named exports

[hedgepigdaniel]

Summary

See #245

Currently to create a loadable component, the module that is asynchronously loaded must export the component as the default export (or be a commonjs module exporting the component as module.exports). This is problematic/limiting because:

• There are good reasons to avoid default exports (prevents grepping a codebase to find usages of an export, prevents import auto completion from working in most/all IDEs)
• It is not possible to make multiple loadable components using the same split point (instead one must hard code chunk names with webpack "magic comments" - not a standard feature)

Detailed choices

See #245 for preliminary discussion of the API

• loadable and loadable.lib still accept the import creator as the first argument. This means that for the common/simple case of loadable(() => import('./xyz')), there is no breaking change, instead options are added.
• resolveComponent is optional and defaults to the existing behaviour. Again, no breaking change, but typescript type inference (for the props of the loadable component) does not work unless it is specified. I think this is a consequence of the CommonJS compatibility.

Benefits

Ability to use named exports (including on the server side)

Use resolveComponent to select the export

Ability to create multiple loadable components from a single split point

// split.tsx
import React from 'react';

type Props1 = {
  prop1: string;
};

export const Component1 ({ prop1 }: Props1): JSX.Element => (
  <span>{prop1}</span>
);

type Props2 = {
  prop2: string;
};

export const Component2 ({ prop2 }: Props2): JSX.Element => (
  <span>{prop2}</span>
);

import loadable from '@loadable/component';

const LoadableComponent1 = loadable(() => import('./testComponent'), {
  resolveComponent: (mod) => mod.Component1,
});

const LoadableComponent2 = loadable(() => import('./testComponent'), {
  resolveComponent: (mod) => mod.Component2,
});

resloveComponent has access to props

import loadable from '@loadable/component';
const DynamicLoadableComponent = loadable(() => import('./xyz'), {
  resolveComponent: (mod, props) => mod[props.export],
});

Typescript type inference

When using resolveComponent, typescript is able to infer the type of the Props of the loadable component (previously they were always unknown). I think the default type/implementation of resolveComponent breaks type inference somehow by supporting commonjs default exports.

Before

// testComponent.tsx
import React from 'react';

type TestProps = {
  prop1: string;
};

export default ({ prop1 }: TestProps): JSX.Element => (
  <span>{prop1}</span>
);

// loadableTestComponent.ts
import loadable from '@loadable/component';

// Inferred type: `LoadableComponent<unknown>`  :(
const LoadableComponent = loadable(() => import('./testComponent'));

After

// testComponent.tsx
import React from 'react';

type TestProps = {
  prop1: string;
};

export default ({ prop1 }: TestProps): JSX.Element => (
  <span>{prop1}</span>
);

// loadableTestComponent.ts
import loadable from '@loadable/component';

// Inferred type: `LoadableComponent<TestProps>` :)
const LoadableComponent = loadable(() => import('./testComponent'), {
  resolveComponent: (mod) => mod.default,
});

Test plan

Existing tests have been changed where necessary and new tests have been added for new functionality

[hedgepigdaniel]

Amended types: https://github.com/DefinitelyTyped/DefinitelyTyped/compare/master...hedgepigdaniel:loadable-named-exports?expand=1

[NT-RX0]

Will this feature be in next release ?

[theKashey]

• there should be some updates to the readme/site about the new feature
• watchdog sounds a bit strange for me. So it is a secondary promise resolver, which might do nothing more, but delay loadable resolution a bit, but only in "async" mode. It sounds more like a condition or guard. For example, I am using something like it to "delay" lazy component resolution till extra .css files, previously "critically inlined" got loaded.

[hedgepigdaniel]

• there should be some updates to the readme/site about the new feature

I've added documentation to the website to go with each commit

• watchdog sounds a bit strange for me. So it is a secondary promise resolver, which might do nothing more, but delay loadable resolution a bit, but only in "async" mode. It sounds more like a condition or guard. For example, I am using something like it to "delay" lazy component resolution till extra .css files, previously "critically inlined" got loaded.

I thought of "watchdog" because its like a parallel process that supervises the loading process and may intervene if it goes wrong. "guard" also sounds fine to me, so I've updated it to be called "guard". That probably fits better with delays (the guard took a while to let the promise through) vs timeouts (the guard gave up waiting for the promise and rendered the error state).

[hedgepigdaniel]

Also, I've put the commit that changes the Babel plugin last.

So if you wanted to delay the breaking change, this could be released in two steps:

1. New features resolveComponent and guard, updates to documentation to stop recommending wrapping the import promise and instead recommend guard for delays and timeouts
2. Breaking change: the Babel plugin makes an error if the import() is wrapped

[theKashey]

This would "fix" some user code. By breaking the compilation.
The error should point somewhere to loadable readme with a brief explanation of a problem and a way to resolve it - resolveComponent

[hedgepigdaniel]

I added a link in the error message to the notes for loadFn, which in turn link to the docs for guard, delay/timeout, and resolveComponent

[theKashey]

according to this code, resolveComponent should not return yet another promise. And I reckon someone would try to.
Could we check that Component is ReactIs.isValidElementType?

[hedgepigdaniel]

Done. That prevents you from using resolveComponent with loadable.lib, but then there is no point doing that (since you get the entire module anyway).

[hedgepigdaniel]

Also I had to bump up the maximum bundle size

[theKashey]

You have got the approval from me. I like the change to the API, and I like the implementation. @gregberge - please take a look.

[gregberge]

Hello guys! @hedgepigdaniel @theKashey

First, thanks for your work on this feature! It is great!

I think we could make the API more generic and more powerful. I explain my idea:

import loadable from '@loadable/component'

const Loadable = loadable(() => import('./components'))

// this function will only be called on client-side, it can be asynchronous
Loadable.getContext = async ({ props }) => {
  // we actually load data dynamically with the component
  const book = await getBook(props.id)
  // we could also add a default delay
  await delay(200)
  // we are able to change the component
  return { books: { [props.id]: book  }
}

// this function has to be synchronous, it is called on client and server
Loadable.resolve = ({ $component, $module, props, ctx }) => {
  // we got some data from context
  const data = ctx.books[props.id]
  // we are able to change the component
  return { $component: $module.Foo, props: { ...props, data } }
}

<Loadable id="10" />

On the server, we will be able to pass a context:

const  book = await getBook(req.params.id)
const ctx = { books: { [req.params.id]: book }  }
const extractor = new ChunkExtractor({ statsFile, ctx })

If Suspense is available one day, resolveContext could be called on server-side. For now it is not possible.

What do you think?

[gregberge]

Oups I closed it!

[theKashey]

• Data loading is great, but it should be like a "single point of truth". Ie getContext should be also called - on the clientside, as well as on SSR.
• Data loading is important, and it could work "this" way after Suspense, but right now it has to be finished before the render, and bound to the routing.
• getContext might use some hooks(useContext) to provide data for a component, as well as suspense features in the future, but in sync way, so it should be more compatible with today.

[hedgepigdaniel]

hey, thanks for taking a look at this.

What do you think?

• getContext is first defined on a particular loadable component, but on the server side it's passed to the entire react tree? This seems imconsistent.
• Is Loadable.resolve the part that can resolve named exports? If so, how is it possible that it receives the resolved component as a prop already?
• having options assigned imperatively (e.g. Loadable.resolve = ...) will make it hard for type inference to know the type of the loadable component

Having a way of passing extra props to the component seems neat, but I'm not sure how to do it consistently without suspense. If you have to set up 2 different methods for SSR and client side, I suspect using an external state management instead (e.g. Redux + route based data loading) would work better.

[gregberge]

You are right. Data fetching is another problem, not our business here.

Your implementation is nice. I am wondering if guard is enough explicit or not.

[theKashey]

Well, let's think what it is:

• it is a guard(as seen in type guards, or more like xstate guards, which prevent state transition) - it might stop something
• it is a check - it might perform some operation, or wait for some operation before resolving import promise
• it a timeout - it might reject, causing component to fall into the error state
• it is only for clientside, only for async, and it also could be async
• and we don't care about the result, only about "open" or "close" mode.

It could be a guard, it could be a gateway.

Another question - why we might use it:

• to emulate time-in, like "flash of loading content". The better option would be to use useTransition, which is not yet "official", but roughly reflect how one should handle a situation like this.
• to emulate time-out, but it could be done in another, more suspense friendly, way. Like Suspense fallback might change some state after timeout, causing Application to display an error state.
• to wait for some "external" condition. I personally use it with critical CSS extraction - like before "resolving" split code I have to "reinject" slit css. A quite rare case.

Read: there is a better way, there is a better way, it's better to use a global guard for stuff like this(read - there is a better way).

---

So what about just not mark this option as an experimental? I am pretty sure not many people were using this functionality so far, as long as they would had their SSR broken, and React.lazy does not provide anything like this, and people are "ok" with it...

[hedgepigdaniel]

Another option is to have options more specifically tailored to common use cases. For example:

• Add built in minDelay and timeout options (probably 95% of the use cases for guard)
• rename guard to something related to loading extra modules/requests... e.g. loadExtra or sideload (probably 95% of the remaining use cases)

Marking it as experimental or simply not adding the option is also fine (and therefore not making the babel plugin complain basically the first 2 commits only). That still makes named exports possible, which is the most important thing.

My thinking in including that change here is that it is confusing that the current loadFn works differently on the server side - delaying the promise works as you would hope, but changing its return value fails spectacularly. That is a trap for users, and trying to use named exports with the current API is one way you can fall into that trap, as in #245.

[theKashey]

rename guard to something related to loading extra modules/requests

Which would not be called if this components was SSRed and loaded in a "sync" way, so please don't put any valuable logic inside. 😒

From a customer point of view it would be great to have just a global option to control minDelay/timeout, both of them could be "fixed" from the outside of Loadable component. In a bit harder way, but could.

[gregberge]

Re 4 - I didn't know it was possible to specify a timeout in import() - how do you do that?

https://webpack.js.org/configuration/output/#outputchunkloadtimeout

Hard to answer without looking at the use case I think - I guess its useful if as the developer you know that there a a few components (small unique code size for each one) that users usually use all/many of them, or none at all. That way it might be a good idea to make the first chunk slightly bigger and eliminate the delay for the other components. I did add an example in this PR (something to do with apples and oranges - but at least it illustrates the idea of having one split point with multiple components. Do you think that's enough, or it should be something else?

Naming imports are not a good idea because you don't have Tree Shaking. We have to warn about it on the documentation.

As soon as I have time, I will merge this one and work on it. Last week-end I was working on SVGR. One project per week-end :).

[theKashey]

Naming imports are not a good idea because you don't have Tree Shaking.

That's not quite bound to this case. It's more about - you will not get treeshaking if you have more that one thingy exported from a module, and more that one thingy consumed.
But, if it's always one, and always use through the loadable - why not to use default?

[gregberge]

But, if it's always one, and always use through the loadable - why not to use default?

Yeah, the point is maybe we just have to "not support named exports". Creating a module wrapper is not a big deal and it is a good practice for tree shaking.

@hedgepigdaniel I am interested to have your advice on this.

[hedgepigdaniel]

Ok, so it sounds like we are trying to work out what is the use case for named exports. Here's a few use cases (using a module wrapper with a default export breaks these use cases):

1. Project where named exports are used for DX reasons

Default exports encourage the same thing to have different names in different files. This is confusing, prevents easy refactoring, prevents good support by tooling (e.g. import auto complete), and adds to cognitive load. See this short and sweet post for more info.

2. Connecting multiple loadable components to the same split point

Take an example that there is a modal that the user may open, and the modal that has tabs inside it. Users usually explore all the tabs after they open the modal. There are 5 tabs. 2 tabs are visible to all users, but 3 tabs are only accessible to administrators.. The content of the tabs is reasonably small (not much data to download), but each request still adds latency.

In this case, it might be a good optimization to use the same split point for all of the 3 components used in the administrator tabs. That way if a user opens one of the tabs, the code for all the tabs is downloaded, so there is no extra latency when they switch to other tabs.

It's not possible for webpack to determine which split points will be loaded at the same time at runtime, so allowing multiple named exports in a single points allows the user to give webpack that information so that it can make a more optimal choice of which code to put in which chunks.

There seems to be a few users interested in this ability: #245 (comment)

---

To put it really simply, javascript supports named exports, webpack supports named exports, so loadable-components should support named exports. The fact that it currently doesn't is surprising and inconsistent with the rest of the ecosystem.

Re tree shaking, it seems like its a fundamental limitation with the current javascript that import() does not support statically importing a particular export - it always returns the entire module. I think this problem is out of scope for this PR (and this library). The user is free to use 1 default export, 1 named export, or multiple named exports. The user writes the code that does import('./xyz'), so they are responsible for what that does. This problem already exists anyway - even though the user must use a default export with loadable-components, they can still add more (unused) named exports to the imported module if they want, and tree shaking will not remove them. Perhaps one day there will be an import async { Thing } from './module';.

[theKashey]

Ok. So in short - the main advantage for named exports is the ability to use them "more often" without overthinking the problem and extra boilerplate.
Ie just use.

[gregberge]

@hedgepigdaniel the chunk name resolves the second issue. Webpack groups chunks with the same name.

Also the case you mentioned is not the nominal one, I think most of time users will use it for wrong reasons and just lose the tree shaking.

[hedgepigdaniel]

No, chunk names does not resolve the issue. Chunk names lets you force webpack to put things in the same chunk, but:

• it prevents webpack from combining that with other chunks if it sees fit (as it does, to make the best compromise the number of requests vs the amount of data downloaded).
• That feature is a non standard webpack specific protocol, and it uses JSON inside comments which is awful.
• That means the user is responsible for choosing unique chunk names, which is insane

The idiomatic way to have multiple things behind a single split point is to export multiple things from one module, with named exports. No non standard features, no hacks to force a specific compiler to do what you want, just using the standard features of the language.

[hedgepigdaniel]

You only "lose" tree shaking to the extent that you put multiple named exports in one file anyway - but loadable-components is currently broken even with a single named export. The use cases are already explained in the original PR description and the linked issue. This is clearly causing pain and confusion (and bugs in production), and there is a simple fix right here. @gregberge It's been almost 4 months now since this PR was opened, and you've already said twice in this thread that you will merge it as it is, but every 2 weeks you change your mind. How much longer do we have to continue this discussion?

[gregberge]

@hedgepigdaniel since I am not very involved in the project it takes time and I am sorry for that. Yeah it is broken but we have two choices:

• Make it work
• Make it impossible

https://reactjs.org/docs/code-splitting.html#named-exports

"React.lazy currently only supports default exports." The currently makes me think that it is not definitive.

Let's make it possible. Also @hedgepigdaniel are you interested to be more involved in the project? Maintainer?

[hedgepigdaniel]

This project? I thought you were the author of it?

Sure, I'm happy to help with maintaining. HMU if you want to chat about it.

[gtrufitt]

Morning @gregberge @hedgepigdaniel thanks for your work on this - reading through the thread it seems like this has been ready to ship for a while. From a consumer perspective, it'd be great to see that happen as it's probably a blocker for us considering we'd have to refactor our project to use default exports.

[theKashey]

Meanwhile...
Long story short but at the "real work project" we are using only names imports. And just a few days ago I've got a good example of exporting multiple entities from one chunk.

• there is a Page
• the Page defines data query above it, so it could load js in a parallel with data
• the Page contains a subroute, which initialization code is a part of the Page
• but that code would be executed only after Page loads and that might take a while.

So - if you are landing to a subroute, you have to "see" how parent page is loading.

The right fix for the problem

Refactor all routing, so the problem will not exists.

The "faster" fix for the problem

Hoist Route a bit above, and "pick" one more export from the chunk. The case is solved, roughly 1 minute for all.

• In other words - named exports, as well as multiple imports from one file, are very refactoring friendly.
• In other other words - we should not bound our decisions to "React", as long as codesplitting is just a generic module management pattern. As long as we are not using the same module system as Facebook - their decisions might not be very applicable to our webpack world.

👍 make it possible.

[lkaratun]

Would love to see this merged soon! I really want to use the package in my org's codebase but we have a ton of named exports

[smoothie99]

Hi @hedgepigdaniel and @gregberge what is the plan for this?

[zeppelinen]

I also would love to see it merged in the main tree.

[theKashey]

Then let's do it!
(reading PR again)

[clgeoio]

+1, would love to see this merged, happy to help fix any blockers

[kyarunts]

+1, looking forward to seeing this merged as soon as possible :)

[theKashey]

• to double check how this plays with loadable.lib

[theKashey]

Ok. So the only missing piece is to update definitely-typed

[kelly-tock]

where are the docs for this? I don't see it on the site anywhere?

[hedgepigdaniel]

https://loadable-components.com/docs/api-loadable-component/#optionsresolvecomponent

[kelly-tock]

Ah yes thanks!