qri-io / 2017-frontend

qri electron & web frontend
https://qri.io
MIT License
23 stars 7 forks source link

need codesplitting to prevent editor from being complied into readonly app #441

Closed b5 closed 5 years ago

b5 commented 5 years ago

lib/routes.js needs some attention, possibly with an update to our babel/webpack config. But currently the compiled readonly main.js is ~5.6mb. I'm assuming it would be much smaller without the editor included.

ramfox commented 5 years ago

Right now, our webapp does not know about or have a 'read only' mode. It asks the backend for a sessionProfileID and if it makes decisions about what to display based on whether or not sessionProfileID is empty (and empty sessionProfileID means read-only). The backend determines whether or not the frontend displays in read-only. Which means the webapp right now needs to come equipped with all the code paths/modules that gets used in normal mode, but does not get used in read-only.

In order to bundle different versions of the webapp, we need to have the notion of read-only exist in the frontend codebase.

One version of doing this includes creating a new webpack file that contains a READONLY environment variable, and we check against this variable when we establish routes/add links/load sections of a page that should not be loaded in read-only mode. This webpack file also does not include loading any monaco node module files.

However, do we still want to include checking against a sessionProfileID for read-only mode? Keeping this would allow someone to turn on read-only mode on their local node and use the webapp/electron app without error.

Speaking of electron, do we want a renderer read-only webpack file as well?

It seems like practically, we only need a read-only version of a webpack file that relates to the productionalized and dev version of the webapp

b5 commented 5 years ago

One version of doing this includes creating a new webpack file that contains a READONLY environment variable, and we check against this variable when we establish routes/add links/load sections of a page that should not be loaded in read-only mode. This webpack file also does not include loading any monaco node module files.

I think this is the way to go for now. Moar webpack == 😢, but I think it's the right thing to do in this case. Totally agreed the way we should handle codesplitting in our codebase is with if (__BUILD__.READONLY) clauses.

However, do we still want to include checking against a sessionProfileID for read-only mode? Keeping this would allow someone to turn on read-only mode on their local node and use the webapp/electron app without error.

I think we should do as little splitting as possible, so yes I think we should include the check and restrict the use of __BUILD__.READONLY to only one use case: only use the READONLY flag to restrict which components are bundled into the readonly app.

That means we should not use READONLY to cut up business logic, or go down different codepaths, only to cut pieces of the app out. I think anything else is an antipattern that'll lead to trouble.

Speaking of electron, do we want a renderer read-only webpack file as well?

Nah. Electron unpacked is ~154MB. an extra 4 isn't going to kill us right now. Let's save those types of optimizations for the future.