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

Server side rendering have no effect in 3.0.x #128

Closed
dxx opened this issue Oct 30, 2018 · 20 comments
Closed

Server side rendering have no effect in 3.0.x #128

dxx opened this issue Oct 30, 2018 · 20 comments

Comments

@dxx
Copy link
Contributor

dxx commented Oct 30, 2018

Hi!
I have writen a ssr demo using 2.2.3,here is my demo.
Now i want to upgrade loadable-components from 2.2.3 to 3.0.x,only a few things have been done

  1. Upgrade babel to 7.x.
  2. Replace all import Loadable from "loadable-components"; with import Loadable from "@loadable/component";.
  3. Replace babel plugin loadable-components/babel with "@loadable/babel-plugin".
  4. Using dynamic mode in src/server.js replace old api

.

server.js

const ReactDOMServer = require("react-dom/server");
const { LoadableState } = require("@loadable/server");
// ...

let component = createApp(context, req.url, store);

const loadableState = new LoadableState();
const loadableComponent = loadableState.collectChunks(component);
// ...

let html = ReactDOMServer.renderToString(loadableComponent);

let htmlStr = template
    // some code
    .replace("<!--react-ssr-outlet-->", 
    `<div id='app'>${html}</div>\n${loadableState.getScriptTags()}`);

After call ReactDOMServer.renderToString(),i call loadableState.getScriptTags(),but return <script>window.__LOADABLE_STATE__ = [];</script>

Thanks

@gregberge
Copy link
Owner

Hello @code-mcx, can you create a branch with your changes, I will take a look.

@gregberge
Copy link
Owner

You must use loadable and not Loadable keyword, the babel plugin only applies on loadable. I think I will change it to read it from import X from '@loadable/component'.

@dxx
Copy link
Contributor Author

dxx commented Oct 31, 2018

Hi @neoziro , i change Loadable to loadable as you say. The lazy loaded component can render in server-side, but loadableState.getScriptTags() still return <script>window.__LOADABLE_STATE__ = [];</script> and i noticed that the root element in App.jsx has no data-reactroot property after render, so react give me Warning: Did not expect server HTML to contain a <div> in <div>.

When i use 2.2.3, i use LoadableComponent.Component to get the lazy loaded component.

const asyncData = route.component.Component.asyncData;

How i get the loaded component in 3.0.x, because i define a static method that used to fetch data in the component.

Here is my branch with problem.

@gregberge
Copy link
Owner

Hello @code-mcx, I gave a look and detect several problems:

  • Webpack is not up to date, please upgrade to v4
  • You are not using https://github.com/liady/webpack-node-externals, so there is two instances of React and Loadable Components, not good
  • About asyncData, this is not solvable in the new version because the "import" function is no longer static, it means that the same Loadable Component can load two different components.

@dxx
Copy link
Contributor Author

dxx commented Oct 31, 2018

Hi @neoziro , I noticed that loadable-component use context api to share the instance of LoadableState. I had move the code of dynamic mode to entry-server.js to make the same instance. I had upgrade webpack to v4 and loadable-components to v4, then i will try. My routes configuration as follow

const router = [
  {
    path: "/bar",
    component: loadable(() => import("../views/Bar"))
  },
  {
    path: "/baz",
    component: loadable(() => import("../views/Baz"))
  },
  {
    path: "/foo",
    component: loadable(() => import("../views/Foo"))
  },
  {
    path: "/top-list",
    component: loadable(() => import("../containers/TopList")),
    exact: true
  },
  {
    path: "/top-list/:id",
    component: loadable(() => import("../containers/TopDetail"))
  }
];

When i visit /baz or /foo, everything is ok, but visit /top-list, i got some error, like picture

1
2

I run build for production, start the server, then visit /baz, cant not found js

3

Please take a look the new code, change is here
Thanks

@gregberge
Copy link
Owner

There was some issues I fixed in #130, please first upgrade to v4.0.2. Several issues persist:

  • I think top-list issue is an issue relative to asyncData, I think you can access it directly on component because static properties are hoisted: route.component.asyncData
  • The second problem is the usage of chunkhash, it does not work, just use hash instead

Please tell me if it works for you!

@gregberge
Copy link
Owner

Your issue with /top-list will be fixed by #131, it was a bug with webpack common chunk configuration. The plugin now disable it for "loadable" chunks. It should fix your issue.

@gregberge
Copy link
Owner

Also, webpack plugin is now always required.

@dxx
Copy link
Contributor Author

dxx commented Oct 31, 2018

Thanks. Tomorrow morning, I'll try.

@dxx
Copy link
Contributor Author

dxx commented Nov 1, 2018

Hi @neoziro . I upgraded @loadable/component to v4.0.2, @loadable/server to v4.0.4, @loadable/babel-plugin to v4.0.3, and using @loadable/webpack-plugin v4.0.4, then solve the issue with /top-list. I made some change:

  • Using hash instead chunkhash in production

  • Moving asyncData to routes configuration

Change

I also found a problem that when i run in production mode, first visit /top-list, the style of component is disappeared. I just using MiniCssExtractPlugin in production. This problem happend on server-side. When visit another url without style of component, then visit /top-list on client-side, the style loaded normally.

Now the new version disable webpack common chunk and must use hash instead chunkhash. I think those two features is useful for production

  • Common chunk like the package in node_modules folders or the js file we wrote doesn't need to change for a long time. We can extract it for browser cache and reduce the size of main bundle.

  • The chunkhash can indicates whether the contents of the file have changed. Every chunk has their own chunkhash, chunk change, file name also change, the browser can load new file. Unchanged chunk
    keep cached locally. But hash applies on all chunks, one chunk changed, all chunks changed.

I hope the new version can support those tow features.

@gregberge
Copy link
Owner

Yes I know that these two features are very important and I plan to support it.

About your CSS problem, this library is only focused on JS part. It does not support ant CSS Code Splitting. If you want to Code Split CSS, I suggest you to use a CSS in JS library.

@gregberge gregberge added the bug label Nov 6, 2018
@gregberge
Copy link
Owner

SSR has been completely rebuilt in v5, chunk hash is supported and CSS is also supported! Can you please give it a try? 🙏

I close this issue because it is now outdated.

@dxx
Copy link
Contributor Author

dxx commented Nov 13, 2018

Hi @neoziro . I tried this new version. It works well for my problems.
When i call chunkExtractor.getLinkTags to get "preload" links, the souce map does not filtered.

20181113153118
20181113153219

@gregberge
Copy link
Owner

@code-mcx oh, it looks like a filter is missing, I will fix it thank you!

gregberge added a commit that referenced this issue Nov 13, 2018
Source maps must no be included in links.

See #128
@gregberge
Copy link
Owner

@code-mcx fixed and published

@dxx
Copy link
Contributor Author

dxx commented Nov 13, 2018

Thanks.

@dxx
Copy link
Contributor Author

dxx commented Nov 14, 2018

Hello @neoziro . I think the initial script shouldn't have async property. Because it has some impact.

First i run the app in development, then change some code to trigger HMR, the browser add a new js file. When i refresh the browser, sometimes the browser throw an exception.

20181114123708

I guess that the new file need execution orderly. Maybe can use defer instead. What do you think?

@gregberge
Copy link
Owner

Can you give me your exact configuration, I would like to know what is going on. The issue looks like to be linked with Hot Reloading.

Loading the main js file without async is not very good for performance. I think we could find a solution without affecting performances.

@dxx
Copy link
Contributor Author

dxx commented Nov 14, 2018

Look at here.

@gregberge
Copy link
Owner

@code-mcx I see the problem, I think we should ignore "hot-update.js" files, they should not be added into scripts but only available to be loaded by Webpack Hot Loader runtime.

I will try to fix it soon. Thanks for your feedback.

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

Successfully merging a pull request may close this issue.

2 participants