react-router v4 && react-hot-loader@3 && Code Splitting by ZeroCho · Pull Request #919 · reactGo/reactGo

ZeroCho · 2017-08-09T15:36:12Z

@choonkending @slavab89

ZeroCho · 2017-08-09T15:37:17Z

@GGAlanSmithee #841 #902

ZeroCho · 2017-08-09T15:37:44Z

#908 @ramakay

Cleanshooter · 2017-08-09T16:21:36Z

I'll do a fresh pull to night and check this out! <-- excited 😄

GGAlanSmithee · 2017-08-09T19:57:25Z

@ZeroCho Thanks for the ping! Very exciting, I will take a look at it as soon as I get some free time. Without having looked at it - does this PR do server-side rendering + code splitting with react-router@4? If so, that's very nice, I have struggled with it a lot. Been a bit intrigued by https://github.com/faceyspacey/redux-first-router, wanting to test it out, have you come across it?

ZeroCho · 2017-08-10T15:15:44Z

@GGAlanSmithee Yes. SSR + Code splitting with react-router@4
I didn't have time to comment on my code, and didn't test it, so errors may happen

I haven't used redux-first-router

choonkending · 2017-08-10T21:46:57Z

@ZeroCho This is an excellent!

I like the fact that we are no longer relying on react-router-redux any longer. Decoupling our navigation from redux was one of the aims I had in mind.

I would like to spend some time this weekend playing around further with this branch in order to have an understanding of what's changed. Perhaps some modules can be split into more, mainly because they seemed to be doing too many things. But I would have to try it out in order to give a better comment!

ZeroCho · 2017-08-11T02:46:54Z

@choonkending Decoupling react-router-redux is quite easy. If you want me to decouple it. I will.

However, I like react-router-redux because it dispatches LOCATION_CHANGE action everytime url changes. By listening to the action, I can simulate onUpdate of react-router@3, which is in app/client.js. Sadly there's no more onUpdate feature support in RR4

choonkending · 2017-08-12T00:14:06Z

package.json

-    "webpack-manifest-plugin": "^1.1.0"
+    "style-loader": "^0.18.2",
+    "url-loader": "^0.5.9",
+    "webpack": "^3.5.2",


Wow! @ZeroCho - You've also updated webpack. Bloody champion

slavab89 · 2017-08-12T16:04:03Z

So i've started to play around with it on my side and i'm getting

Warning: setState(...): Can only update a mounting component. This usually means you called setState() outside componentWillMount() on the server. This is a no-op. Please check the code for the AsyncComponent component.

I presume that this happens because of the promise there.. componentWillMount exists and when the promise returns setState is called.

How would it be possible to disable the async component loading on the server side? Doesnt seem to be necessary anyway, no? Maybe with something like https://github.com/thejameskyle/react-loadable ?
Basing on this https://gist.github.com/acdlite/a68433004f9d6b4cbc83b5cc3990c194

Edit
While trying to make react-loadable work i had to do several changes:

Change asyncComponent to work with react-loadable instead of the code that's present there.
install babel-plugin-import-inspector and add a plugin:

['import-inspector', { serverSideRequirePath: true, webpackRequireWeakId: true }]

exclude 3 libraries from externals based on this https://twitter.com/mostruash/status/884194310769565697

const externalModules = fs.readdirSync('node_modules')
  .filter(x => ['.bin'].indexOf(x) === -1 && x !== 'react-loadable' && x !== 'is-webpack-bundle' && x !== 'webpack-require-weak')
  .reduce((acc, cur) => Object.assign(acc, { [cur]: 'commonjs ' + cur }), {});

I still received the same error and then i stumbled upon jamiebuilds/react-loadable#54 . Tried to make the change on the local library and the error was gone 😄 . I now wonder when and if it will be merged or if there is a better solution.

ZeroCho · 2017-08-13T10:26:26Z

~~@slavab89 Changing from componentWillMount to componentDidMount will resolve that problem.~~

~~And it is for Search Engine Optimization.~~

GGAlanSmithee · 2017-08-15T07:46:34Z

Been testing this a bit and it seems to be working fine, with the small aside mentioned by @slavab89. I definitely agree that code splitting on the server should be avoided if possible. I have no idea how to do this with AsyncComponent though. Using webpacks chunk plugin, i managed it with a hack;

new webpack.optimize.MinChunkSizePlugin({minChunkSize: browser ? 10000 : Infinity})

@ZeroCho

And it is for Search Engine Optimization.

Care to elaborate on this? Not sure what you are reffering to exactly.

ZeroCho · 2017-08-15T08:24:36Z

@GGAlanSmithee Ah, I now got what @slavab89 meant. I had some misunderstanding. I think I can change some code of app/routes.js to avoid code splitting on the server.

component: isServer ? Home : asyncComponent(...)

And about Warning: setState(...): Can only update a mounting component. This usually means you called setState() outside componentWillMount() on the server. This is a no-op. Please check the code for the AsyncComponent component.
It's very hard to remove this message. It's just a warning but it is annoying.

slavab89 · 2017-08-15T18:22:04Z

So i've just discovered something that might bring us the solution..?
First of all, the article: https://medium.com/faceyspacey/code-cracked-for-code-splitting-ssr-in-reactlandia-react-loadable-webpack-flush-chunks-and-1a6b0112a8b8
It is actually pretty interesting.
And the module itself (Pretty much react-loadable successor) https://github.com/faceyspacey/react-universal-component
And they've create a demo as an example: https://github.com/faceyspacey/universal-demo

Did anyone of you heard about this?

GGAlanSmithee · 2017-08-23T07:55:10Z

app/containers/Dashboard.jsx

-const Dashboard = () => <div>Welcome to the Dasboard. Stay tuned...</div>;
+class Dashboard extends Component {
+  componentWillMount() {
+    const { user: { authenticated }, history } = this.props;


Is it possible to couple this with route config as in rr3? Or is this the new way of protecting routes? If so, maybe this should be in it's own component (or HoC)? Usage would be like

<Protected redirect={'/login'}> <MyComponent/> </Protected>

Here is an example of a HOC doing this:

import {Component} from 'react' import PropTypes from 'prop-types' import {connect} from 'react-redux' import {Redirect} from 'react-router' const asProtected = (WrappedComponent, selectData) => { class Protected extends Component { static propTypes = { user: PropTypes.object, history: PropTypes.object } state = { redirect: false } componentWillMount() { const {user: {authenticated}} = this.props if (!authenticated) { this.setState({ redirect: true }) } } render() { if (this.state.redirect) { const {history} = this.props const location = { pathname: '/login', state: { nextLocation: history.location } } return ( <Redirect to={location}/> ) } return <WrappedComponent {...this.props}/> } } const mapStateToProps = ({user}) => ({ user }) return connect( mapStateToProps )(Protected) } export default asProtected

Usage:

import asProtected from 'hoc/asProtected' const MyComp = () => ( <div> Only for logged in users! </div> ) export default asProtected(MyComp)

This will always redirect to the login route., but could be changed to take the redirection path as an argument if needed I guess.

The original location (to the route which is protected) will be stored in the location.state.nextLocation object, so that it can be used to redirect after succesfull login.

GGAlanSmithee · 2017-08-23T07:59:11Z

app/routes.jsx

-};
+}
+
+export default [{


a bit sad to see the jsx route trees go, was visually intuetive too me, but I guess that's the way the cookie crumbles.

GGAlanSmithee · 2017-08-23T08:05:20Z

Took a brief look at the code. Not too much to add unfortunatley, it looks fine too me.

The only real issue discovered so far is the server-side code splitting if I understand it correctly? It might be inconveniant, but it's not really a bug. This PR seems like the way to go. rr4 and react-hot-loader 3 is 👍. Maybe if it's merged, we can iterate on it? Is there anything besides time blocking it? 😄

choonkending · 2017-08-29T07:55:31Z

Hey peeps, just an update, Ive been working on a branch inspired by @ZeroCho 's work here. It's still in WIP (lots to cover) master...choonkending:react-router-4-clean.

I thought there might be a better way around our server side and client side data fetching, so I was experimenting around with that.

Just to let you peeps know that I'm not ignoring this awesome PR!

slavab89 · 2017-09-11T07:05:31Z

So i've recently tried to use https://github.com/faceyspacey/react-universal-component to achieve SSR + Code Split. I can say that its really easy to do it with it. Just follow their demo.
The thing is that it still doesnt play super nice with RR4 (Which seems to be the standard 🤔 ) because of store and history syncing, but they also have https://github.com/faceyspacey/redux-first-router which works with it.
Basically they have a whole stack of projects intended for all of this.

What do you guys think? Or do we have some progress on this?

choonkending · 2017-09-11T09:09:48Z

@slavab89 I'm still on my spike branch based off this one master...choonkending:react-router-4-clean which I think has an acceptable solution to rr4, but without code splitting yet. I was thinking that could be tackled once we have a solution for this first! :D

GGAlanSmithee · 2017-09-12T12:16:49Z

app/client.jsx

+    <AppContainer>
+      <Provider store={store}>
+        <ConnectedRouter history={history}>
+          <PendingNavDataLoader routes={routes} store={store}>


should this be

<PendingNavDataLoader routes={r} store={store}>

?

Good catch. You're right

GGAlanSmithee · 2017-09-12T12:20:28Z

webpack/rules/javascript.js

-  };
-  const presets = createPresets(enableHotModuleReplacement);
-
+  const presets = ['es2015', 'react', 'stage-0'];


should this be

const presets = [ ['es2015', {modules: false}], 'react', 'stage-0' ]

to allow for webpack dead code elimination?

@GGAlanSmithee Does that work in client environment too?

@ZeroCho I don't see why it shouldn't. The webpack bundling (and therefor code elimiation) happends at "compile" time, so it shouldn't matter in what environment the final code is used. Is there any specific reason that makes you think it wouldn't work in browsers?

@GGAlanSmithee You're right. I checked out that it works in client too

react-router v4 && react-hot-loader@3 && Code Splitting

a6da3e9

ZeroCho mentioned this pull request Aug 9, 2017

Code Splitting? #908

Closed

ZeroCho mentioned this pull request Aug 9, 2017

Trying to replace the old HMR... (DO NOT MERGE) #880

Closed

ModuleConcatenationPlugin

d8c8e04

choonkending reviewed Aug 12, 2017

View reviewed changes

GGAlanSmithee reviewed Aug 23, 2017

View reviewed changes

GGAlanSmithee reviewed Sep 12, 2017

View reviewed changes

GGAlanSmithee mentioned this pull request Oct 7, 2017

React 16 #925

Closed

slavab89 mentioned this pull request Oct 17, 2017

Bundle splitting #280

Closed

ZeroCho closed this Feb 27, 2020

enterstudio mentioned this pull request May 13, 2022

[Snyk] Fix for 1 vulnerabilities enterstudio/reactGo#53

Open

Conversation

ZeroCho commented Aug 9, 2017

Uh oh!

ZeroCho commented Aug 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZeroCho commented Aug 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Cleanshooter commented Aug 9, 2017

Uh oh!

GGAlanSmithee commented Aug 9, 2017

Uh oh!

ZeroCho commented Aug 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

choonkending commented Aug 10, 2017

Uh oh!

ZeroCho commented Aug 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

slavab89 commented Aug 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZeroCho commented Aug 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GGAlanSmithee commented Aug 15, 2017

Uh oh!

ZeroCho commented Aug 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

slavab89 commented Aug 15, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GGAlanSmithee Sep 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GGAlanSmithee Sep 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GGAlanSmithee commented Aug 23, 2017

Uh oh!

choonkending commented Aug 29, 2017

Uh oh!

slavab89 commented Sep 11, 2017

Uh oh!

choonkending commented Sep 11, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZeroCho Sep 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ZeroCho commented Aug 9, 2017 •

edited

Loading

ZeroCho commented Aug 9, 2017 •

edited

Loading

ZeroCho commented Aug 10, 2017 •

edited

Loading

ZeroCho commented Aug 11, 2017 •

edited

Loading

slavab89 commented Aug 12, 2017 •

edited

Loading

ZeroCho commented Aug 13, 2017 •

edited

Loading

ZeroCho commented Aug 15, 2017 •

edited

Loading

GGAlanSmithee Sep 14, 2017 •

edited

Loading

GGAlanSmithee Sep 14, 2017 •

edited

Loading

ZeroCho Sep 12, 2017 •

edited

Loading