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

Support route chunking #128

Open
declanelcocks opened this issue Feb 23, 2017 · 17 comments
Open

Support route chunking #128

declanelcocks opened this issue Feb 23, 2017 · 17 comments

Comments

@declanelcocks
Copy link

declanelcocks commented Feb 23, 2017

Is it in your plans to support chunks for individual routes? For example, when you land on /, you only load what you need for /.

I took your repo and added this in for my own project so if you'd like me to help make a PR for this I'm happy to do that. I'm not 100% sure that my implementation is the best or most efficient as I tried to copy the implementation from other projects and my own research, so I would be happy for you to help review and improve it. And of course, I'm not as familiar as you are with the internals of this project, especially the server-side rendering.

In short, I changed all of the routes to use getComponent to allow webpack to create a chunk for each route, that's only loaded when you visit that route. I also had to change the way the client/server renders the app.

Also, I'm not sure on how you like PRs to be structured for this repo, since you have several trees for each progressive feature.

@declanelcocks declanelcocks changed the title Support full chunking Support route chunking Feb 23, 2017
@diegohaz
Copy link
Owner

Hey, @declanelcocks

I've already tried to do that, but I couldn't get it working mostly because of the way we are importing components (require.context). As we import every component at once, webpack wasn't creating another chunks as their code was already added to the main chunk.

I would love to see your solution though. Even if we decide to not add that to the boilerplate, it'll be very helpful for other people as a reference. About PRs, don't worry, I just ask to target the base branch of the feature you want to add (in this case, master).

@declanelcocks
Copy link
Author

@diegohaz Ah yeah, that was the big road block I ran into as well. Everything was working, but it was still just creating 1 chunk because every component/container was being exported from the index.js file. I will try and think of a workaround for this, maybe there is a simple alternative to require.context because it would be a shame to lose the ability to import the components so easily.

@diegohaz
Copy link
Owner

Particularly I don't have problems with it, the gain is enormous, but there's another issue: #113.

We'll probably need an alternative for those who prefer to don't use require.context because of its drawbacks.

I plan to add a npm run setup script to handle things like this (as well as optionally getting rid of react-router, storybook etc.).

@declanelcocks
Copy link
Author

declanelcocks commented Feb 23, 2017

Ah yeah, I took at the differences here that you linked in that issue. Those changes look good; it would be slightly more annoying to create your own components but if you used the generator-arc branch it would be fine. Alternatively, you could introduce something like plop to generate new components instead so that it takes care of remembering to add in exports and imports everywhere.

Are you planning to merge in those changes any time soon or are you looking for an alternative export solution?

I can make a PR for what I've done so far, but keeping the existing require.context. Of course, it will create one big chunk with all of your components but at least the logic is there for review.

@diegohaz
Copy link
Owner

diegohaz commented Feb 23, 2017

That would be fine for the tooling issue, but will not solve the problem with chunking since we would be still importing all components.

Feel free to make the PR. Also, thank you for the feedback.

@declanelcocks
Copy link
Author

Yeah, but I think the biggest pain point with the structure you posted is that you have to remember to add the export to that folder's index file. I only suggested something like plop because you can let plop take care of remember these things.

@zentuit
Copy link

zentuit commented Mar 21, 2017

I think we should look at the concept of the React Fractal Structure. Basically each chunk would have its own components, containers, store, etc. This way each chunk/route would be self contained. Any "global" components would go in the root component.

I think importing from w/in the chunk would go something like
import MyComp from 'route/[route directory]/components'.

@ajhool
Copy link

ajhool commented Mar 21, 2017 via email

@zentuit
Copy link

zentuit commented Mar 21, 2017

Oh I completely understand that @ajhool, its a big 👍 from me for that too. However the fractal structure above just makes a giant component heap for each chunk independently (theoretically - I haven't tried it) with a giant heap for the site wide components.

The path comes into play only at the chunks (ie routes if using fractal structure)
(looks like my structure above was stripped of some characters)
import MyComp from 'route/[route-path]/components'
MyComp is still in either atoms, molecules, etc within that route

(I've noticed I'm using route & chunk interchangeably... under fractal structure, the route definition defines the chunk so the route is the chunk.)

@declanelcocks
Copy link
Author

@zentuit Wouldn't you potentially end up with a longer initial load? Since the site wide chunk could end up including a lot of unnecessary components? In arc for example, you have quite a lot of components in the atoms folder which would get bundled together (as well as things like Header) into one big site wide chunk.

@rolele
Copy link

rolele commented Jun 23, 2017

I do not understand everything but I think this guy found a solution he explain here:
https://medium.com/@apostolos/server-side-rendering-code-splitting-and-hot-reloading-with-react-router-v4-87239cfc172c
and implemented it in its great website here: https://github.com/LWJGL/lwjgl3-www

@declanelcocks
Copy link
Author

@rolele The problem I was referring to was not really the same as this. What I was trying to achieve was to be able to do import { Atom } from 'components' without having to import every single component in the folder. If you try to code split the app right now, you'll only end up with 1 chunk since you're importing all components every time due to the index.js file.

@rolele
Copy link

rolele commented Jun 25, 2017

Do you mean that to be able to perform code splitting we have to include the component using the path components/atoms/Atom (without using import { Atom } from 'components' index.js that export all of them at once)? It makes sense actually.

@declanelcocks
Copy link
Author

@rolele Yep exactly, if you use just 1 component and check your chunks you'll see that it includes every component

@Inoir
Copy link

Inoir commented Aug 25, 2017

https://github.com/ctrlplusb/react-async-component

Really simple to implement in Arc

@velopert
Copy link

velopert commented Oct 5, 2017

Is there any solution to this issue? Should I give up route based code splitting in ARc?

Using directory structures like routes/[route-name]/components/atom/Something.js will solve the issue, but it is too complex

@Geczy
Copy link

Geczy commented Oct 5, 2017

Yeah i'm about to switch all my code to using directory structures

import x from ../../components/atoms/x

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

No branches or pull requests

8 participants