DeloitteAU / react-habitat

⚛️ 🛅 A React DOM Bootstrapper designed to harmonise a hybrid 'CMS + React' application.
Other
262 stars 42 forks source link

Components not loading asynchronously. #28

Closed ryanpcmcquen closed 6 years ago

ryanpcmcquen commented 6 years ago

I probably have something configured incorrectly, but despite using registerAsync:

containerBuilder
    .registerAsync(System.import("./components/Button/Button"))
    .as("Button");

My bundle files are the same size regardless of whether I have one component on the page or all of them.

Here's my manifest file: https://github.com/ryanpcmcquen/react-habitat-poc/blob/master/source/Manifest.js

Any ideas what I might have configured incorrectly? Or is this just how registerAsync works in this scenario?

jennasalau commented 6 years ago

Hi Ryan,

Are you using webpack? My guess is that your Button component has a small lines of code number so webpack is being smart in this case and saying its better to keep it in the main bundle rather than take on the extra HTTP download overhead.

You can configure this setting using the MinChunkSizePlugin. For example, I set this to a ridiculous 1 character limit in our example to force it to create new chunks.

ryanpcmcquen commented 6 years ago

@jennasalau, thank you. I have added the minChunkSizePlugin but it seems the bundle size still doesn't change between having only the Card component (https://ryanpcmcquen.org/react-habitat-poc/card.html): screenshot 1516656922

And having the Card, Cart, and ProductCard component (https://ryanpcmcquen.org/react-habitat-poc/): screenshot 1516656909

jennasalau commented 6 years ago

Hi Ryan,

Finally had a chance to look into this one. You have indeed found a bug as I can replicate it.

It seems the way we have architected the registrations, the Promise body is evaluating immediately causing the chunks to load upfront.

Currently we are supporting code separation which was tested, just not lazy loading :(

The problem seems to be with the Habitat API at the very surface level.

containerBuilder.registerAsync( new Promise() )

The promise executor is evaluating immediately (or in our case resolving the module/chunk).

We will probably have to wrap it in a function that returns a Promise so it only evaluates the executor when called.

ie

containerBuilder.registerAsync(() => { return new Promise() } )

Unfortunately this is going to be a breaking change dangit.

ryanpcmcquen commented 6 years ago

I'm sad that it will be breaking, but very excited to see this working! It is essential for us. 😸

jennasalau commented 6 years ago

I have pre-released this fix as part of v1.0.0-beta (Since we have been sitting stable for a while lets put this as a v1 milestone).

To test this fix please npm install react-habitat@1.0.0-beta --save-dev I will be getting our internal project teams to do the same.

ryanpcmcquen commented 6 years ago

npm install react-habitat@1.0.0-beta --save?

jennasalau commented 6 years ago

I guess it depends if you're building a JS library or an app :D

Webpack will put React Habitat into the built binary so no need for the dependency after that.

ryanpcmcquen commented 6 years ago

Oh, the README has just --save, that is why I asked.

jenssogaard commented 6 years ago

@jennasalau just implemented 1.0.0-beta in an existing project and adjusted my syntax. Works as expected 👍

ryanpcmcquen commented 6 years ago

Working here too: https://github.com/ryanpcmcquen/react-habitat-poc

ryanpcmcquen commented 6 years ago

Is 1.0.0 ready to go stable?

jennasalau commented 6 years ago

Yep it's appearing that way. Just need to test typescript and we can go prod.

jennasalau commented 6 years ago

Tests complete, this is now published as 1.0.0