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

loadable not working in SSR with webpack module federation - #640

Closed
ajayjaggi opened this issue Oct 14, 2020 · 99 comments
Closed

loadable not working in SSR with webpack module federation - #640

ajayjaggi opened this issue Oct 14, 2020 · 99 comments
Assignees
Labels

Comments

@ajayjaggi
Copy link

🐛 Bug Report

loadable-components: failed to synchronously load component, which expected to be available { fileName: './src/shared/dedicated/index.js',
chunkName: 'Dedicated',
error: 'Cannot read property 'call' of undefined' }
(node:7562) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'call' of undefined
at webpack_require (/Users/ajay/Desktop/WebStrom/MicroFrontend/Edge/dist/server/main.js:394:42)
at Module../src/shared/components/Footer/index.js (/Users/ajay/Desktop/WebStrom/MicroFrontend/Edge/dist/server/Dedicated.server.js:21:71)
at webpack_require (/Users/ajay/Desktop/WebStrom/MicroFrontend/Edge/dist/server/main.js:394:42)
at Module../src/shared/dedicated/index.js (/Users/ajay/Desktop/WebStrom/MicroFrontend/Edge/dist/server/Dedicated.server.js:60:76)
at webpack_require (/Users/ajay/Desktop/WebStrom/MicroFrontend/Edge/dist/server/main.js:394:42)
at Object.requireSync (/Users/ajay/Desktop/WebStrom/MicroFrontend/Edge/dist/server/src_server_render_js.server.js:225:14)
at InnerLoadable.loadSync (/Users/ajay/Desktop/WebStrom/MicroFrontend/Edge/dist/server/vendors-node_modules_loadable_server_lib_index_js-node_modules_express_index_js-node_modules_-ee7ccd.js:420:35)
at new InnerLoadable (/Users/ajay/Desktop/WebStrom/MicroFrontend/Edge/dist/server/vendors-node_modules_loadable_server_lib_index_js-node_modules_express_index_js-node_modules_-ee7ccd.js:315:17)
at processChild (/Users/ajay/Desktop/WebStrom/MicroFrontend/Edge/dist/server/vendors-node_modules_loadable_server_lib_index_js-node_modules_express_index_js-node_modules_-ee7ccd.js:56603:14)
at resolve (/Users/ajay/Desktop/WebStrom/MicroFrontend/Edge/dist/server/vendors-node_modules_loadable_server_lib_index_js-node_modules_express_index_js-node_modules_-ee7ccd.js:56568:5)
(node:7562) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:7562) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

To Reproduce

routes.js -

import React from 'react'
import loadable from '@loadable/component'
// import Home from './home'
// import Dedicated from './dedicated'

const Home = loadable(() => import(/* webpackChunkName: "Home" / './home'))
const Dedicated = loadable( () => import(/
webpackChunkName: "Dedicated" */ './dedicated'))

const homeRoute = (path) => ({
path,
exact: true,
component: Home
})

const dedicatedRoute = (path) => ({
path,
exact: true,
component: Dedicated
})

export default () => [
homeRoute('/'),
dedicatedRoute('/:player(messi)')
]

Expected behavior

A clear and concise description of what you expected to happen.

Link to repl or repo (highly encouraged)

https://github.com/ajayjaggi/MicroForntEnd-Basic-Structure

Issues without a reproduction link are likely to stall.

@open-collective-bot
Copy link

Hey @ajayjaggi 👋,
Thank you for opening an issue. We'll get back to you as soon as we can.
Please, consider supporting us on Open Collective. We give a special attention to issues opened by backers.
If you use Loadable at work, you can also ask your company to sponsor us ❤️.

@ajayjaggi
Copy link
Author

ajayjaggi commented Oct 14, 2020

Hi. i saw this PR https://github.com/gregberge/loadable-components/pull/638/files and used these fixes to run loadable in my local along with webpack 5. But further in SSR, wrapping components with loadable breaks the flow...Error is published above.
Also i have mentioned my repo link..

@StephaneRob
Copy link
Contributor

StephaneRob commented Oct 15, 2020

@ajayjaggi
Copy link
Author

ajayjaggi commented Oct 15, 2020

Thanks for this.I used the wrong key.
Have corrected this but still getting the same error.

@StephaneRob
Copy link
Contributor

Did you use webpack LoadablePlugin in your webpack config?

@ajayjaggi
Copy link
Author

@ajayjaggi
Copy link
Author

ajayjaggi commented Oct 15, 2020

https://github.com/ajayjaggi/MicroForntEnd-Basic-Structure - this is the PR for project.
The error of 'Cannot read property 'call' of undefined' is occurring only in server side rendering.For client side everything is working fine

@ajayjaggi ajayjaggi reopened this Oct 15, 2020
@theKashey
Copy link
Collaborator

It may take a while before it will start working properly, but 🤞

@ajayjaggi
Copy link
Author

Hi @StephaneRob / @theKashey ,can we connect over a call to discuss the issue ? We can try to find the solution together.

@theKashey
Copy link
Collaborator

@ScriptedAlchemy is the boss in MF lands. We all need his help, not my help.
PS: #635 sounds to be related. We need unique callbacks for every remote. Or we don't?

@ScriptedAlchemy
Copy link

Each host has a name. Webpack usually appends a name to the remote containers. So they get their own name.

Webpack accesses the runtime via window.app1.get.
App1 is the name you call it in the config.

I'm not sure if jsonp matters because they all get namespaced. It would have to be tested but I use loadable and do not have problems. However I'm using MF not standard entry-point runtimes which execute differently.

@ScriptedAlchemy
Copy link

Quoting from a comment I left on recently merged PR:

If you have the remotes on the page immediately. Like hardcoded. All remote code should transport down in a single RTT.

If you want to SSR the chunks a remote requires:

loadable needs a new way to map names of remotes to federated chunk maps.
I need to extend the MF api so remotes will export their chunk maps so loadable can extract them during render.
Whenever you use loadable. Babel basically transforms the import into a object with a normal require statement and the module / chunk id. Then it's executed the HOC pushes these IDs into a scope that the server can access after the react render.

The only Id it will have is ./someExposedModule - and we would need to go over the remotes stats and find what the browser side script is for that module.

Same problem exists in next js. The loadable method straight crashes.
No existing code split tool is capable of MF chunk coronations. I work around hydration issues with top level awaits. Which work on both client and server.

You could also use partial hydration and simply hydrate markup when visible.

I don't foresee this being hard to accomplish. Especially since I can extend MF apis at will.

We could even change the getter on the remote so whenever a server gets code from the remote, it's container will report what request was asked for from the remote. It actually could be done without and need for Babel at all, or even loadable HOC. The only part we would need to adapt is ChunkExtractor to read from the second scope the remote containers are accessing directly.

This mechanism would work exactly like react-universal does. Push right into a map, but we will be doing this inside the webpack runtime directly.

MF should not be a complex problem to render correctly on. You don't need to use loadable as a wrapper 👆 we only need to flush chunks into it from webpack runtime.

@ScriptedAlchemy
Copy link

@theKashey how can I import and push extra chunk names into your context.

How can we have loadable read the remote containers chunks. I can have webpack infer this from the plugin.

What we could do is provide a loadableFederationPlugin which passes the object to MF but we can read the info.

Then we would have the paths to all remotes and can require the chunk stats upfront.

I could write to a map you expose directly in webpack. As the runtime attaches, it sends context to the host. The "hard" part on the webpack side is emitting the stats and reducing it down to simple maps.

If I'm able to push chunk maps and the module as it's required at runtime. This would make it much simpler. Then I wouldn't need to provide stats upfront but could push them into loadable as the remote attaches on its own.

@ScriptedAlchemy
Copy link

Okay, I'm on my iPad to cant really code much. Here's what the startup code will look like. (Check module federation examples/startup-code) to see it configured in webpack.

This entrypoint would be part of webpack-imported, and its added to webpack with something like SingleEntryPlugin or EntryPlugin (or mutation of options, like u do in next)
We can get app1 from webpack and use template strings, or we can compose variables via definePlugin, like you do to env vars.

Global.chunkMap = Map() // or whatever
const container = __webpack_require__('webpack/container/entry/app1');
// app1 could be process.env.CURRENT_HOST or something we can inject during build. 
Module.exports = {
  get(request) {Map.add(request); return container.get(request)}
  init() {return container.init(arguments)}
  ClientChunks: non_webpack_require('../clientStats.json') // or some injection to inline this after the fact. 
}

We can also use normal require and stuff in here, its just a entrypoint.

As i use federated import(scope/request), RemoteModule will perform global.[scope].get(request)

Since we own the getter, its going to push into a global map. We can then read config from MF plugin to get paths to remote container if we need.

ImportedFederatedPlugin({
Name:app1
Remotes: {otherRemote: path.resolve(pathToRemote)}
})

Now you've got the options passed to MF. So we can infer name, and know how to find the container to get the exported client chunks off it.

You can also use, in the server. So chunkExtractor would go:

remoteChunks = webpack_require(require.resolve(remote)).ClientChunks

Alternatively, we could attach to scope during initialization

{init()=>{Object.assign(global.chunkMaps,ClientChunks)}}

Inside loadable, you'd pretty much want to prevent the typical babel transform. Instead we use webpack directly. We can use require if needed for server transform, then webpack will hoist to app boot time, i think.

Lastly, we could use the existing HOC to push ones actually rendered, depending on if the require function pull the chunks right away. Ideally if we can preloadAll or something it might be better.

There's likely some details to work out but this will work.

@ajayjaggi
Copy link
Author

hi @ScriptedAlchemy, i was very curious about finding the solution to the problem. If we could talk and you could guide me through, i would be very happy to contribute.

@ScriptedAlchemy
Copy link

There's other issues that block webpack support besides this :/

But I'll push a branch somewhere soon that you can look at

@ajayjaggi
Copy link
Author

Hi @ScriptedAlchemy , If you could share the branch it would be great help

@ScriptedAlchemy
Copy link

I'm building most of this capability into the module federation dashboard.

I'll backport so capabilities to standalone

@stale
Copy link

stale bot commented Dec 31, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Dec 31, 2020
@visormatt
Copy link

visormatt commented Jan 4, 2021

@theKashey Can we re-open this guy as it's a blocker for users of this package from updating to Webpack 5? Also is there any idea of timing or any assistance we "the community" can help with? CC. @ScriptedAlchemy

@stale stale bot removed the wontfix label Jan 4, 2021
@theKashey
Copy link
Collaborator

webpack 5 support was already PR-ed - #676, I am waiting for a little free time to get it merged.

@visormatt
Copy link

Amazing, I should have poked around the branches a bit more, I didn't see it linked here 🤷. Thanks again and thank you for all the fantastic work thats gone into this project!!

@AlexSergey
Copy link

webpack 5 support was already PR-ed - #676, I am waiting for a little free time to get it merged.

Any progress?

@pauliusuza
Copy link

Any progress on this? Seems to break razzle builds (hash mismatch on production build)

@suhanw
Copy link

suhanw commented Mar 12, 2021

Watching this issue as well.

I also created a reproduction repo. README has instructions on how to run locally.
https://github.com/suhanw/wp5-modfed-ssr-loadable

A very simple setup where remote exposes TopNav component, and host consumes TopNav.
https://github.com/suhanw/wp5-modfed-ssr-loadable/blob/main/host/client/components/App/index.js

const TopNav = loadable(() => import('remote/top-nav'), { ssr: true });
The line above throws the following error:

loadable-components: failed to synchronously load component, which expected to be available {
  fileName: 'webpack/container/remote/remote/top-nav',
  chunkName: 'remote-top-nav',
  error: '__webpack_modules__[moduleId] is not a function'
}
(node:62803) UnhandledPromiseRejectionWarning: TypeError: __webpack_modules__[moduleId] is not a function
    at __webpack_require__ (/Users/suhanwijaya/Desktop/temp/wp5-modfed-ssr-loadable/host/build/server/bundle.js:249:41)
    at Object.requireSync (/Users/suhanwijaya/Desktop/temp/wp5-modfed-ssr-loadable/host/build/server/server_renderer_js.bundle.js:60:14)
    at InnerLoadable.loadSync (/Users/suhanwijaya/Desktop/temp/wp5-modfed-ssr-loadable/host/node_modules/@loadable/component/dist/loadable.cjs.js:278:35)
    at new InnerLoadable (/Users/suhanwijaya/Desktop/temp/wp5-modfed-ssr-loadable/host/node_modules/@loadable/component/dist/loadable.cjs.js:173:17)
    at processChild (/Users/suhanwijaya/Desktop/temp/wp5-modfed-ssr-loadable/host/node_modules/react-dom/cjs/react-dom-server.node.development.js:3305:14)
    at resolve (/Users/suhanwijaya/Desktop/temp/wp5-modfed-ssr-loadable/host/node_modules/react-dom/cjs/react-dom-server.node.development.js:3270:5)
    at ReactDOMServerRenderer.render (/Users/suhanwijaya/Desktop/temp/wp5-modfed-ssr-loadable/host/node_modules/react-dom/cjs/react-dom-server.node.development.js:3753:22)
    at ReactDOMServerRenderer.read (/Users/suhanwijaya/Desktop/temp/wp5-modfed-ssr-loadable/host/node_modules/react-dom/cjs/react-dom-server.node.development.js:3690:29)
    at Object.renderToString (/Users/suhanwijaya/Desktop/temp/wp5-modfed-ssr-loadable/host/node_modules/react-dom/cjs/react-dom-server.node.development.js:4298:27)
    at _callee$ (/Users/suhanwijaya/Desktop/temp/wp5-modfed-ssr-loadable/host/build/server/server_renderer_js.bundle.js:134:76)

import TopNav from 'remote/top-nav';
This line above appears to work, but extractor.getStyleTags() returns empty string (this seems related to #706)

@ajayjaggi curious if you found a workaround?

@GitProdEnv
Copy link

Any progress?

I get the same error

loadable-components: failed to synchronously load component, which expected to be available {
  fileName: './src/components/Mfa_1_app.tsx',
  chunkName: 'components-Mfa_1_app',
  error: '__webpack_modules__[moduleId] is not a function'
}
(node:29690) UnhandledPromiseRejectionWarning: TypeError: __webpack_modules__[moduleId] is not a function
    at __webpack_require__ (/media/gitprodenv/Elements6/Coding/mfa/packages/container/dist-backend/webpack:/@microfrontend/container/webpack/bootstrap:19:1)
    at Object../src/components/Mfa_1_app.tsx (/media/gitprodenv/Elements6/Coding/mfa/packages/container/dist-backend/dev-server-bundle.js:3086:73)
....

@IMalyugin
Copy link

I believe I found a working approach, going to compile a solution soon. Not sure if it's a right place for it, but during loadable constructor calls (aka loadable(...)), it's possible to collect a global object with [chunkName]: importAsync calls. Then loadableReady will wait for Promise.all([...]), for each external chunk listed in __LOADABLE_REQUIRED_CHUNKS___ext.namedChunks

@IMalyugin
Copy link

IMalyugin commented Mar 21, 2022

Okay, got it working both server- and client-side.

Got a question on which strategy do we take server-side?
Choice A: Pass extra option to babel-loader to inject static import (ssr: true), so that entire loading is handled by webpack bootstrap
Choice B: Make loadableReady call required on the server. And then use same resolution strategy as the web, except it also requires a reverse-reverse lookup. For some reason I need to preload same components from different remotes.

@IMalyugin
Copy link

IMalyugin commented Mar 21, 2022

@theKashey @ScriptedAlchemy Uploaded a PR with loadableReady turned into an async bootstrap.

A lot of things still need to be tested, but it works in my setup. Also I did not actually "build" loadable yet. Just modified it inside node_modules. Then accurately copied the solution to the repository. Will soon check if the built version is working properly.

I also didn't manage to collect all the needed chunks for wmf bootstrap + shared + loadable. Aka if I wrap ./bootstrap with loadable it throws on shared not available for eager consumption. If I don't wrap it, bootstrap chunk is not registered with loadable. That has been reported before #690

@theKashey
Copy link
Collaborator

There is a little unknown moment around the way loadable names chunks - it has to be deterministic and the names has to be unique.
#885 seems to be capable resolving the root cause, but:

  • federated chunks will not be flushed during SSR, and I am not sure if it's even possible as due to WMF nature those names are not "static"
  • all federated modules will be loaded, regardless are used or not, leading to undesired results.

@IMalyugin
Copy link

IMalyugin commented Mar 22, 2022

There is a little unknown moment around the way loadable names chunks - it has to be deterministic and the names has to be unique.
#885 seems to be capable resolving the root cause, but:

That can be listed in caution for wmf. chunks should probably include [remoteName] section in webpack. We can also try binding resolution to manifest files generated by webpack, if the problem arises. I’ll try production builds, to see if the mapping is done right. Added to todo list.

  • federated chunks will not be flushed during SSR, and I am not sure if it's even possible as due to WMF nature those names are not "static"

Don't see a problem here, we render the them all together, so they should be properly flushed.

  • all federated modules will be loaded, regardless are used or not, leading to undesired results.

I only request chunks gathered during SSR and passed to named chunks, so it shouldn't be a problem. Worth checking if that is true thou. Added to todo list.

@IMalyugin
Copy link

IMalyugin commented Mar 22, 2022

Lol, found the problem with shared modules, its kinda laughable.
I declared React as shared in WMF. And loadable is using react internally. So It jamms into chicken/egg paradox.

WMF requires bootstrap to load eager dependencies, such as React. And loadable must wrap bootstrap, to gather chunks, which requires react. On top of that, loadable is only meant to be used inside components, so webpack bootstrap pretty much breaks the concept.

--- edit

So far, I don't see how loadable can consume WMF`s import('./bootstrap'). That could become a show-stopper. If that's what you mean under not working federated chunks flushing, then I pretty much understand it now

@IMalyugin
Copy link

IMalyugin commented Mar 23, 2022

We currently have two blockers, each with it's own proposed solution:

Problem 1. WMF eager static imports are generating chunks that aren't flushed by loadable.

Solution: Webpack uses async import boundaries to take its time and preload all the eager static imports. Meaning the dependencies are kept somewhere. WMF turns them into Promise.all like this:
image
We can extract them with webpack-plugin somehow and attach as dependencies to their async boundaries chunks. @ScriptedAlchemy can probably point us in the right direction - how to properly pinpoint eager static imports binding.

Maybe we could patch webpack to add eager import dependencies to compilation.stats? If it is not already possible by mapping chunk children to origin[].moduleId starting with "webpack/sharing/consume", like:
image


Problem 2. Top level async boundary - bootstrap is not flushed. Loadable is meant to process react components, there were no need to support regular imports. So we can't just loadable(() => import('./bootstrap')) if there is no react tree rendering, we can't extract the chunk.

Solution: basically "bootstrap" or top level async boundary is a synchronous pattern, there should be no "conditional" rendering to it compared to loadable components, thus we can just tell loadable to treat that chunk as one of those always required dependencies. To do so, I suggest introducing loadable.bootstrap(() => import('./bootstrap'))


So far all of this seems doable.

--- edit 1

Spent whole day, trying to build some dependency graph for shared eager module from compilation.stats.chunk.origins, then from raw compilation.stats.modules, but I believe that is impossible, unless, well parsing module "webpack/sharing/consume/**" nested reason->children fields to find dependencies.

@ScriptedAlchemy
Copy link

I parse the MF plugin options, so i know everything about that plugin and if its shared or not.

I track what's loaded per request, so i dont load anything additional other than the execution tree. Preloading only 3-4 files that were actually used.

For vendors, in webpack they have a shared module type, so i can look at the stats of graph and find all the ids, chunks they live in, how its referenced.

This is a large portion of the magic that makes it work in next.js - might be some applications here, at least conceptually. The module id is a placeholder id, so you have to look past that key and know what's the chunk associated with it.

Async import bootstrap also solved many issues, but you'd still want to flush chunks out of SSR for maximum performance

https://gist.github.com/ScriptedAlchemy/7c1c7b25665524fbb0dfb4f06db7ebff

@ScriptedAlchemy
Copy link

These shared modules, you'd want to know what came from where. I do this by tracking the initialization scopes which include what, and from who.

@IMalyugin
Copy link

IMalyugin commented Mar 24, 2022

That's actually great news, meaning I'm on the right track. Or even on the right paved road!

I parse the MF plugin options, so i know everything about that plugin and if its shared or not.

Got that part from webpack stats as well, it has all the info about consumes and provides

I track what's loaded per request, so i dont load anything additional other than the execution tree. Preloading only 3-4 files that were actually used.

That's the magic done by loadable, don't really plan to take that bread, loadable tracks rendered components very well, only need to figure out how to let it track async initialization.

For vendors, in webpack they have a shared module type, so i can look at the stats of graph and find all the ids, chunks they live in, how its referenced.

That's what I spent whole yesterday doing, and only by the end of the day discovered there is a "reason" section inside webpack/sharing/consume "modules", that tells who uses what.

This is a large portion of the magic that makes it work in next.js - might be some applications here, at least conceptually. The module id is a placeholder id, so you have to look past that key and know what's the chunk associated with it.

Yep, that's the info I also got from the webpack.stats

@ScriptedAlchemy
Copy link

Parsing reasons is pretty common.

Another thing to watch for is if it's shared code, you want to put it in the right parents in the map. So like react, I'd push its chunk into the array of files needed at entry point level. Otherwise things could double load.

@IMalyugin
Copy link

IMalyugin commented Mar 24, 2022

Parsing reasons is pretty common.

Another thing to watch for is if it's shared code, you want to put it in the right parents in the map. So like react, I'd push its chunk into the array of files needed at entry point level. Otherwise things could double load.

Did not reach that part yet.

I planned to extract shared provides and shared consumes by chunks, and put them into loadable-stats. After that find a way to require shared without duplication. That approach will also simplify mf orchestration, since I can join multiple loadable-stats and dedupe shared even further.

@IMalyugin
Copy link

IMalyugin commented Apr 1, 2022

Okay, been off the radar for a week doing other things, but now I'm back.

Finished working on Problem 1.: extracting WMF shared consumes and attaching them to chunks. Now chunks section is extended with sharedConsumes:
image

That in turn are listed in sharedModules section. All the data is extracted purely from stats.json via loadable/webpack-plugin
image

(Updated PR: #885)

The next step is Problem 2: Telling loadable to extract async modules between entrypoint and App. As soon as I started thinking about that problem I've hit a multiple choice decision.

Prerequisites:

  1. During chunk flushing, we tell wmf to use an entrypoint;
  2. Chunks connected to react components wrapped by loadable are gathered during Server-Side Rendering;
  3. Between 1 and 2 there is an async boundary: export default import('./bootstrap'), needed for WMF to collect eager depencendies;

So, there are at least 3 possible ways to proceed:


Choice 1: Using webpack stats, I can find all the async chunks between entrypoint and every loadable. Assuming the application only has one path from entrypoint, that would allow gathering these async chunks automatically with no extra boiler code;
Problems: what happens if there are more than one conditional path between? And what to consider the application root?
Possible Solutions: We could hypothetically throw a warning that there is more than one possible import path, so extra chunks are flushed and fix. The first chunk registered by ChunkExtractor can be considered the root chunk.


Choice 2: I could add loadable.bootstrap(() => import('./bootstrap')), this way we know exactly which path to take, or do we?
Problems: client and server usually have different entry points. So the bootstrap call flushed on the server will not match the call on the client.
Possible Solutions: It's ugly, but we could attach a key argument for bootstrap call, in case they don`t match (and if there is more than one bootstrap in the entrypoint).


Choice 3: We could have a loadable wrapper at application root, this way we would definitely know which chunk contains the root of application. This solution does not involve much changing to loadable mechanics, since ChunkExtractor can already work well with react components
Problems: This approach will not work if there are multiple async boundaries between entrypoint and application root.
Possible Solutions: Restrict making multiple boundaries? Combine with solutions for 1/2?


I don’t like choice 1 as it becomes uncontrollable and unpredictable. Choice 3 has a rather bad limitation.

So I favor the 2nd choice. We could also combine 2 and 1. Aka use bootstrap to tie down entrypoint with application root. But build the path from entrypoint to the root via webpack stats, following available client-bundle bootstraps.

@IMalyugin
Copy link

IMalyugin commented Apr 3, 2022

Did some analyzing of @loadable/babel-plugin contents and to be honest, I'm lost. From the way it is currently implemented, I am not sure what needs to be done next.

If we agree choice 2 is is the optimal way, we just need to add new loadable.bootstrap function. That function would have to be transformed towards the same path loadable components operate. Thing is, I don't need to actually change the async chunk with a component, not even sure I need to meddle with the chunk name. But I do need to somehow mark that import or bind it to the the react app part. And that binding has to work even if there are no loadable components on the other side.

Tinkering with babel-plugin AST is complicated enough as it is, not to go around experimenting, trying to figure out what has to be done.

One more thing: I don't want to turn loadable-components into imported-components, so these async boundary chunks must be applied during render, not import. Or as other choices state, we can use static chunk tree analysis instead.

@IMalyugin
Copy link

So, there are at least 3 possible ways to proceed:

Choice 1: Using webpack stats, I can find all the async chunks between entrypoint and every loadable. Assuming the application only has one path from entrypoint, that would allow gathering these async chunks automatically with no extra boiler code; Problems: what happens if there are more than one conditional path between? And what to consider the application root? Possible Solutions: We could hypothetically throw a warning that there is more than one possible import path, so extra chunks are flushed and fix. The first chunk registered by ChunkExtractor can be considered the root chunk.

Choice 2: I could add loadable.bootstrap(() => import('./bootstrap')), this way we know exactly which path to take, or do we? Problems: client and server usually have different entry points. So the bootstrap call flushed on the server will not match the call on the client. Possible Solutions: It's ugly, but we could attach a key argument for bootstrap call, in case they don`t match (and if there is more than one bootstrap in the entrypoint).

Choice 3: We could have a loadable wrapper at application root, this way we would definitely know which chunk contains the root of application. This solution does not involve much changing to loadable mechanics, since ChunkExtractor can already work well with react components Problems: This approach will not work if there are multiple async boundaries between entrypoint and application root. Possible Solutions: Restrict making multiple boundaries? Combine with solutions for 1/2?

I don’t like choice 1 as it becomes uncontrollable and unpredictable. Choice 3 has a rather bad limitation.

So I favor the 2nd choice. We could also combine 2 and 1. Aka use bootstrap to tie down entrypoint with application root. But build the path from entrypoint to the root via webpack stats, following available client-bundle bootstraps.

Covering edge cases, I got the following results:

  1. Choice 1. Will NOT work. If we have an app that does not consume any loadable ssr components, but still use wmf shared and async boundary, we will not be able to determine find all the async boundaries between App and entry, because we would have no loadable calls within App;
  2. The above is handled by choice 3 properly, but there is still the limitations of multiple async points. For now the only solution I see is choice 2. Naming async boundaries would both let us support complex cases such as different bootstrap path for client and server and give toolset for manual manipulation.

Going deeper into the current goals for choice 2 solution:

  1. Our main goal is to capture the imported chunk names for all the bootstrap calls;
  2. To capture it during runtime and not compile time, we can proxy import's thenable. When 'then' is called, we add the chunk to the ChunkExtractor;
  3. Proxied then call happens between ChunkExtractor initialization and collectChunks call;
  4. We might require multiple bootstrap calls for client for every bootstrap call on the server and vice versa;
  5. Loadable keeps deterministic naming for all the chunks, in our case, we should give bootstraps special names based on both given key and path. If key exists, we'll be matching part of the chunkName representing the key, otherwise, we check against the path;

So from what I see, we can do something like: LoadableBootstrap__src_components_index_jsx for path-related keys and "LoadableBootstrap_some_key__src_components_index_jsx" for keys. Alternatively, I'm not sure if we need to rename chunks at all. Why can't we just gather them during thenable call with their given names/keys. It's not like we actually need to do the loadable magic for sync/async call later on, they are only needed for the flushing.

I'll spend some more time clearing out why's and how's chunkName replacement works in loadable/babel-plugin and then try to implement that bootstrap utility.

@IMalyugin
Copy link

IMalyugin commented Apr 7, 2022

Got some great news:

  1. I was able to implement the loadable.bootstrap function. Funny enough the function contains but a warning that it should not be called without babel plugin. Babel plugin replaces the call entirely with an iife that returns thenable. Once that thenable is resolved it both pushes the bootstrap chunk to global object stack and calls the async import; Thenable allows us to create onDemand bootstrap. It will only be registered if it is resolved.
  2. Having loadable.bootstrap completely replaced fixes the "chicken-and-egg" problem, initial chunk no longer depends on loadable nor react, so we can separate loadable and react into WMF shared dependencies entirely;
  3. I did some analysis on babel-plugin chunk name generation and bumped into the issue which apparently was already raised before: Babel-plugin: Unrelated files are being bundled together due to import with identical relative paths #549. Posted a simple solution that will fix the collision, also with some work the very same solution may fix collision between different mfs;

What's left to do:

  1. The working loadable.bootstrap tells that a bootstrap chunk is loaded, but there are still no ways to merge them, since there can be multiple bootstrap chunks SSR and CSR. Furthermore, we can't call these bootstraps passing same variable and expect them to match, unless we just pass string literal with bootstrap name;
  2. ChunkExtractor will get an update supporting promise of react-component. This will allow us to flush and rewind bootstrap chunks between renders. There is also a large work related to user-error solving, aka if instead of passing promise to chunkExtractor, developer resolves it prematurely and then passes down the component, registered bootstrap chunks should throw an error;
  3. ChunkExtractor should also learn working with sharedModules that loadable config now provides, replicating WMF shared;
  4. The naming issue above requires resolving;
  5. Need to enhance loadable SSR toolkit to allow joining multiple loadable-stats into a single united stats, so that chunk mechanics works out of the box without external scripts and plugins;
  6. Documentation, optimization, tests, examples, etc.

--- edit 1
Connecting bootstrap from SSR with CSR is not as simple as I thought. For now, I only see the following solutions:
a. Set predefined name e.g.: "bootstrap-loadable" for bootstrap chunk through babel-plugin. For more complicated cases, allow manually adding webpackChunkName, so that multiple bootstraps see each other.
b. By default assume that both server and client use the same bootstrap call, if that is not true, force webpackChunkName usage;
c. This is rather complicated, but instead of using webpackChunkName, we could add a parameter to the import, there is no problem with parsing string literal, but if that parameter is imported from another module, we could try using static analysis;

Solution b. seems most obvious, as it does not obstruct the normal chunk generation flow and uses same mechanics applied by other parts of loadable. Guess I'll start from here.

--- edit 2
The scope keeps getting bigger every time I do something. Figured out that If I gather bootstrap chunks by calling global function, that would create conflict if multiple SSR are running in the same scope, they could potentially interfere with one another. To fix that, I'd have to use node async context, and if it is not supported by the environment, create a mutex (only allow the async execution to complete one at a time.

@IMalyugin
Copy link

IMalyugin commented Apr 13, 2022

Finally finished the async bootstrap chunk registration, It was harder than I thought and required me to do a little hack with unused export const __LOADABLE_MARKER_BOOTSTRAP__${chunkName} = true; marker. I decided to keep things simple: no dynamic bootstrap chunk, if you call loadable.bootstrap(() => import('./path/to/bootstrap')), that chunk will always be bundled with the chunk that imported it.

Now switched to enhancing ChunkExtractor, to make it properly use loadable stats for registering both bootstrap- and wmf shared chunks.


@theKashey ever thought about rewriting loadable-stats, so that it only contained fields required by loadable? We could reduce the load on ChunkExtractor and remove a lot of unnecessary data in the stats file without breaking backwards compatibility (except for those, who process stats manually outside ChunkExtractor). If somebody needs some extra data in loadable-stats, they should opt-in for them via configuration.

Also quite literally holding myself back from rewriting ChunkExtractor towards respect for SingleResponsibility.

@theKashey
Copy link
Collaborator

ever thought about rewriting loadable-stats, so that it only contained fields required by loadable?

I also hold the same idea for a long time - #778 (comment)

It was "reduced" multiple times:

And increased as well:

But we are subject for hurim law and should just create another version of Chunk Extractor with the another "API"/"Goal" set initially ( well, https://github.com/theKashey/webpack-imported )


finished the async bootstrap chunk registration, It was harder than I thought

And probably the same can and should be applied to the loadable-component themselves. I don't feel that the current model is actually clicking with Federation and one might need to find another model.
For example create that new ChunkExtractor and make it a part of ModuleFederation - ie let servers share this information, not to make it somehow discoverable on the frontend.

No changes to the MF is required. "Loadable" should just discover federated modules(you did it) and then discover "Loadable" inside them. Repeat until success.

@IMalyugin
Copy link

IMalyugin commented Apr 15, 2022

But we are subject for hurim law and should just create another version of Chunk Extractor with the another "API"/"Goal" set initially ( well, https://github.com/theKashey/webpack-imported )

It feels overwhelmingly wrong to create another plugin, just to allow loadable to simplify stats. It also feels wrong to create 2 stats files by loadable. So what I had in mind, was major release, with a new flag that allows for minimum stats, eventually switching that flag defaulting to minimal stats.

For now, It feels I'm violating pioneers rule, by adding 2 new methods to already overbloated ChunkExtractor class, I'm leaving something in worse state then it was. Guess that's what open source is like, your every step feels like walking on a mine field :)

There is no problem with current loadable-stats config in terms of data sufficiency. But we could optimize a lot of things on the way. Welp, guess not this iteration.

@theKashey
Copy link
Collaborator

It feels overwhelmingly wrong to create another plugin

Unfortunately, stats generated by loadable as over and misused already. Releasing a major version of plugin and introducing the breaking change in the API might break some integrations.

So this can be done only with recommended replacement, a clear migration guide, and the minimal API surface from day 1, as switching that flag defaulting to minimal stats will introduce another breaking change.


Thus I want to ask you - what exactly you need to change? What if in any circumstances the best way forward is creating a special MF-friendly version of ChunkExtractor and Loadable.bootstrap code?
Do anyone not using MF will benefit from the change or it's actually better to keep things detangled?

@IMalyugin
Copy link

Let's move that discussion to the moment when initial implementation is done in pull request. Need to focus on the task first, then we'll see if some suggestions are worth doing.

As for your questions:

  1. bootstrap will be usable for people not using mf, as it allows binding multiple asynchronous chunks together, some task may require this;
  2. Cleaning up loadable stats size would allow us to reduce size by 2 to 4 times, could be useful to others too;

@IMalyugin
Copy link

IMalyugin commented May 6, 2022

Did not abandon the project :) Just got my hands busy elsewhere.
Got a bit of a progress before I had to switch. Both positive and negative.

First for the bad parts:

  1. Tried out production build, as all the moduleIds got replaced with numeric values, some of the parts stopped working, especially all the checks against federated id's; rewrote a couple of checks to another field (aka identifier), but one of the checks, "isFederated" could not be rewritten. It happens during runtime, but we do not have any info on other ids at that time. So unfortunately, to detect federated chunks, the schema has to be even more complicated: got to collect all the federated chunks during webpack build and output them into the client;
  2. Working with shared wmf dependencies also proved more difficult, than I thought. Instead of blindly collecting all the related chunks, I found out that inside webpack stats chunks: [...] arrays within modules represent all the chunks that include a copy of the module. So now we have to thoroughly analyze which chunks are already accumulated and ignore them. To add another nail, the common chunks must actually match the ones added during server build;
  3. Since we need to match some "unknown" chunk inclusion algorithm, we need to render single entrypoint on the client (without extra preloads), see what WMF includes on it's own. Save that order and then compare it against the order we have with LOADABLE_REQUIRED scripts.
  4. Having this in mind, I decided to go for TDD: added jest-puppeteer, configured it out a bit, ran into a trouble with conflicting configurations, started resolving against jsdom that was already used, till I figured out that jsdom is a better fitting solution for the task. And it actually works well with WMF client-side, I got the scripts that WMF loads, just need to clean up my prototype and turn it into an example inside loadable repository;
  5. And the final thumb down goes with remoteEntry.js, analyzing webpack stats, shared dependencies and files WMF loads, It seems remoteEntry.js is out of all those pictures, so we probably have to require it manually just before requiring any of the remote chunks;

To sumarize, the goal yet again got pushed away by a few weeks of development :)

@bannik
Copy link

bannik commented May 6, 2022

Any progress on this? Seems to break razzle builds (hash mismatch on production build)

Unfortunately, I just spent half of my day realizing that this happens when you use webpack 5 or above. Once you downgrade back to webpack 4 it functions normally.

@fivethreeo
Copy link
Contributor

Razzle is quite unstable with wp5, needs help with rewrite btw ;)

@stale
Copy link

stale bot commented Jul 10, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jul 10, 2022
@stale stale bot closed this as completed Aug 13, 2022
@ScriptedAlchemy
Copy link

How I got this to work for next.js was to embed the loadable stats into the remote entry’s runtime. So I have get,init,chunks - then at runtime I’ll collect the chunks from each remote that it’s exporting, then merge them with the known loadable manifest.

For module ids. I didn’t have to worry about that, they were registered as the ids from the other container and merging the manifests at runtime provided the lookups I needed.

Ontop of that, I’ve also changed how remote module is loaded, so when a remote modules get or init apis are called, I’ve got hooks into that and can see “who loaded what” so as it’s executing webpack is pushing requests into a queue that I can flush after render by getting the remotes chunk maps with global.remote.chunkMap

@IMalyugin
Copy link

IMalyugin commented Aug 25, 2022

The hook "who loaded what" seems like some new webpack event? Gave this whole thing a little thought and what's really holding the implementaion back - is WMF awesome async boundary, that just loads whatever it needs (hardcoded as Promise.all in resulting bundle). If that thing could be exposed, it would make things much easier.

Also I don't see how that could be done during SSR (executing webpack), as the eager chunks are not the same for csr and ssr.

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

No branches or pull requests