-
-
Notifications
You must be signed in to change notification settings - Fork 380
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.preload return the promise #226
Conversation
I need it to block the transition during the change of location and show progress. |
There is a small but important change, I would ask you to do – "You should always return the same promise". |
This is a function that can be executed by an event such as a click on a link, we can set a loading status before changing pages and make the page change at the end. render() {
const { children, location } = this.props;
const { previousLocation } = this.state;
// use a controlled <Route> to trick all descendants into
// rendering the old location
return <Route location={previousLocation || location} render={() => children} />;
} See https://github.com/bertho-zero/react-redux-universal-hot-example/blob/master/src/components/ReduxAsyncConnect/ReduxAsyncConnect.js#L42 |
I use an array of routes and react-router-config to search for routes that match, I preload these components and execute a static method of each of them, it does not concern children in my case. |
Hello @bertho-zero, thanks for working on it. I have some problem with that. In my point of view, I understand your problem, but are you sure to apply the correct method? For your server-side problematic, you already have a method to do that. Loading components by yourself is not the plan. You should use |
I use the same method as https://github.com/bertho-zero/react-redux-universal-hot-example, everything works perfectly if I add this return. The promise may not be waited, it is not an obligation. However return undefined makes it impossible to wait. I gave all the info in the previous comments. Before rendering the route on the server or before each route change I preload the components. But for that I must be able to wait for the end of the promise. |
This is the last point that makes me stuck on react-loadable |
I am sorry, but you should do it manually. Create a wrapper and expose a |
redux-first-router is a tricky thing. Once "page change event" is dispatched - page is not changed, only a To be honest - it's not so easy with "full-dynamic imports", but something should exist. why this should be in a core - loadable rely on |
I can launch a method called preload but no way to know when this preload is complete? We have a function that launches a promise without possibility of waiting for it? And a different behavior between the client and the server for no apparent reason. |
My case is not an edge-case, I will not display a page that I do not have yet and I would like to display a loading status between the beginning and the end of the loading of the page. |
Displaying a loading status is currently possible, Loadable Components gives you a "fallback" props that is rendered when it is loading. You are taking a wrong path, you can achieve what you want to do but not the way you want to do it. |
You do not want to understand. I do not want to display fallback, but block the transition and display a bar like YouTube does. And I wish I could make a correct server side rendering. What protects the limitation that I wanted to remove in this PR? Apart from making a universal code compatible only with the client. Here is an example: https://react-hot-example.herokuapp.com The code is on github and is rather appreciated, what is wrong? It's a technical choice that I can at least take with react-loadable. Here is another exemple of what you prevent to do: https://youtube.com |
Preload/prefetch in Webpack terms is about loading some eventually. |
@bertho-zero an example of implementation with fallback: https://codesandbox.io/s/6l65nq9k3n. You have to think in a Suspense way to do it, at least if you want to be future proof. |
Suspense is well inside a page, but does not allow blocking the transition for the route components. It is essentially those who are affected by this change. Your example does not correspond to what I want. This is bad practice for you but do not have all the possible cases in mind, and I repeat my question: this limitation is supposed to protect from what? |
I understand @bertho-zero 's need to preload the component before navigating the user away from the route. You can do this in a library like mobx-state-router with beforeEnter/beforeExit, where the library will not allow the router to issue a navigation until these promises have resolved. Awaiting on preload from loadable components here will give a much better user experience, and I also currently do the effect @bertho-zero described using react-loadable. But I think @neoziro 's solution is to use concurrent mode, @bertho-zero can I ask, is this the kind of effect you want? Since it is possible in concurrent mode by just using lazy and Suspense. |
It's still possible to use Suspense/LoadableState to render some component, which would lock some value in a Context So - no, it would not help. |
You should probably open a new issue on React. You will have the problem on Suspense and all data fetching very soon. Anyway, I will not fix this kind of problem in Loadable Components. |
We remain on a promise that is not possible to wait... You impose your way of doing things on everyone rather than opening up the possibilities a little without damaging anything. This egocentric view of things is incomprehensible in the world of opensource. |
" "You should use What do you want to "protect" us with this limitation? I need to mix the react-router transitions and preloading as it seems to me right. |
Workaround:Step 1: yarn add -D patch-package Step 2: "postinstall": "patch-package" Step 3: diff --git a/node_modules/@loadable/component/dist/loadable.cjs.js b/node_modules/@loadable/component/dist/loadable.cjs.js
index d055987..0a055ce 100644
--- a/node_modules/@loadable/component/dist/loadable.cjs.js
+++ b/node_modules/@loadable/component/dist/loadable.cjs.js
@@ -272,11 +272,7 @@ function createLoadable(_ref) {
});
Loadable.preload = function (props) {
- if (typeof window === 'undefined') {
- throw new Error('`preload` cannot be called server-side');
- }
-
- ctor.requireAsync(props);
+ return ctor.requireAsync(props);
};
return Loadable;
diff --git a/node_modules/@loadable/component/dist/loadable.es.js b/node_modules/@loadable/component/dist/loadable.es.js
index c236921..b94489c 100644
--- a/node_modules/@loadable/component/dist/loadable.es.js
+++ b/node_modules/@loadable/component/dist/loadable.es.js
@@ -266,11 +266,7 @@ function createLoadable(_ref) {
});
Loadable.preload = function (props) {
- if (typeof window === 'undefined') {
- throw new Error('`preload` cannot be called server-side');
- }
-
- ctor.requireAsync(props);
+ return ctor.requireAsync(props);
};
return Loadable;
diff --git a/node_modules/@loadable/component/dist/loadable.js b/node_modules/@loadable/component/dist/loadable.js
index 9d19ee1..b2f4620 100644
--- a/node_modules/@loadable/component/dist/loadable.js
+++ b/node_modules/@loadable/component/dist/loadable.js
@@ -271,11 +271,7 @@
});
Loadable.preload = function (props) {
- if (typeof window === 'undefined') {
- throw new Error('`preload` cannot be called server-side');
- }
-
- ctor.requireAsync(props);
+ return ctor.requireAsync(props);
};
return Loadable;
It's sad, but I'm here for so little. Higher up you told me about creating a Wrapper, how to create a Wrapper having access to the ctor to avoid all that? |
But isn't the UX the same? Both would keep the current component/view until it's "ready". |
The more I think about it the more I think you should not solve this by waiting for the component to load @bertho-zero . See this talk from here on: https://youtu.be/6g3g0Q_XVb4?t=930 I think your problem is solveable with the APIs mentioned above, although I do not know there status since that talk is almost a year old. |
I agree with you but I have been using this for years in a professional context and I do not have a plan to change everything at once immediately, I would like to be free to stay like that for the moment. EDIT: All this is under development and I have a few more months of waiting, while I would be able to do the equivalence of that right now with this PR. |
@bertho-zero I think you should fork Loadable Components and maintain your own solution. I move in the direction of React, so this package will probably not evolve in the way you want. |
One day React will make all the loaders obsolete. const [Component, state] = useLoadable(() => import('file'));
if (state.ready) {
return <Component />
});
return "....."; More about ability to use |
How would Suspense fit into that? Why not just let the not ready state be handled by Suspense? |
It's more about coding style. More than coding style exists in one moment of time. No coding style is better than another one. Never thread something new and upcoming as a silver bullet. Lets imagine - you have code splitted a page. And now you are asked to load it. And fill with a data.
You would have 2 things suspended - from loading chunk, and from loading data. Then imagine, that I am writing this from an island in the ocean, and every
The problem - it's much harder now to "throw" a promise (which is Promise.all), as long as you have to cache it (say hello to stateful classes or hooks). You might not to throw a joined promise, but first Then add SSR, when you have to wait for promises outside of a rendering cycle. Or blocking navigation change, as seen on Suspense demo, but using a real router (react-router, RFR, react-navi...). Suddenly simple ways are no longer simple, but harder ways are simply.
|
@neoziro What do you want to "protect" us with this limitation? With React I can do that: function lazyWithPreload(factory) {
const Component = React.lazy(factory);
Component.preload = factory;
return Component;
}
const StockChart = lazyWithPreload(() => import("./StockChart"));
... with an unexplained limitation. React, https://github.com/theKashey/react-imported-component and https://github.com/jamiebuilds/react-loadable do it. |
@bertho-zero so you have three solutions that works in the way you want! That's good. I have my reasons for not returning the promise. |
React is not yet complete, react-loadable is no longer maintained and I have just discovered react-imported-component. And what are these reasons? |
I managed to work around the problem by overloading the // loadable.js
import loadable from '@loadable/component';
export default ( fn, options ) => {
const Component = loadable( fn, options );
Component.preload = fn.requireAsync || fn;
return Component;
}; |
@bertho-zero nice! |
🎉 |
See gregberge#226 for context
This PR allows to wait for the end of the preload.