3DStreet / 3dstreet-editor

3DStreet Editor Repo
https://3dstreet.app
Other
19 stars 3 forks source link

feat: lazy loading to modals #358

Open rostyslavvnahornyi opened 9 months ago

rostyslavvnahornyi commented 9 months ago

Hi @kfarr, The bundle originally is 1.75mb. After implementing lazy loading and code splitting, the bundle size reduced to 1.67MB.

So, is it necessary to add more routes? If so, which routes should be added? Adding routes doesn't reduce the bundle size, instead, it remains the same. Better approach would be to add webpack split chunks, as it provides a similar impact to routing.

There is a root file – inspector, and introducing new routes to the root file adds more problems. Moreover, adding new routes requires adjusting the logic with urls to address and fixing any related bugs, which can be time-consuming.

Maybe is better solution to leverage webpack split chunks instead of introducing additional routes?

kfarr commented 9 months ago

@rostyslavvnahornyi thanks for this feedback. Here are some ideas on how to proceed. Please let me know your thoughts. If it makes sense, then I will need to create some separate tickets for these various items:


My understanding of routes would be that the "right" answer is to have proper routes that match with the user interface and roughly map to modals and move scene UUID to the path instead of hash. So given the structure of https://URL.com/path/#route we could imagine the following routes in the 3DStreet application:

This structure implies a server-side change to use the path for scene ID instead of the hash:

Other considerations:

rostyslavvnahornyi commented 9 months ago

Hi @kfarr. Thanks for your ideas! Waiting for you to create separate tickets.

kfarr commented 9 months ago

@rostyslavvnahornyi I was looking for some feedback from you -- does this plan work? Or are there things I'm not considering? What about the routes -- are those the right routes or are there some missing? What about the fact that toolbar.js includes save / load features -- does that mean that we should extract those elsewhere so they aren't loaded until needed so the #edit route is smaller?

rostyslavvnahornyi commented 9 months ago

@kfarr, the plan is good. I think your point about the toolbar is valid, and we need to move it under the viewer route, but It might be difficult.

The other routes are correct; we've covered almost all aspects of the application with these routes, and nothing has been overlooked.