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

Vue settings apps #9938

Closed
rullzer opened this issue Jun 20, 2018 · 22 comments
Closed

Vue settings apps #9938

rullzer opened this issue Jun 20, 2018 · 22 comments
Assignees
Labels
1. to develop Accepted and waiting to be taken care of enhancement feature: vue apps
Milestone

Comments

@rullzer
Copy link
Member

rullzer commented Jun 20, 2018

Right now the vue settings app already has 2 routes (one for the users page and one for the apps page).

However since we'd like to move more settings stuff (like the apptoken stuff since it still uses handlebars etc) to vue as well. It would be great to find a solution to this issue.

The main problem is that the current router handles different urls fine. However. It it mounted on the #content div. Which is the outer diff.

@rullzer rullzer added enhancement 1. to develop Accepted and waiting to be taken care of feature: vue apps labels Jun 20, 2018
@rullzer rullzer added this to the Netcloud 15 milestone Jun 20, 2018
@nextcloud nextcloud deleted a comment from nextcloud-bot Jun 20, 2018
@rullzer
Copy link
Member Author

rullzer commented Jun 27, 2018

I just realized we'll have this issue more probably when we move less 'isolated' stuff to vue. Like the share sidebar part. And the contacts menu etc. They all live somewhere in core/js.

@skjnldsv
Copy link
Member

@rullzer independant apps do not need routes :)
We can just plug them in where we want them

@rullzer
Copy link
Member Author

rullzer commented Jun 27, 2018

@skjnldsv sure. But it is not about independant apps.

  • authtokens in settings
  • contacts menu in core
  • share sidebar stuff in core (granted they could probably be moved to files_sharing)
  • other stuff in core that still uses handlebar

If we'd make them all small independant apps. We'd get a real complex build script I guess.

@skjnldsv
Copy link
Member

It is, they're just apps that are rendering on a div, they're here on every page :)

  • authtoken
  • contacts menu

The sidebar is also just an app, but needs to be pluggable and usable by others apps
If the #app-sidebar is here, then load the script, render the app and register the plugin!
Basically unless you're creating an app that uses a routes.php we don't need to setup routes on vue so it's easy.

For the settings, let's keep our php and keep registering our components with php so the script is properly loaded. We don't need to convert this to vue I think :)

@ChristophWurst
Copy link
Member

If we'd make them all small independant apps. We'd get a real complex build script I guess.

Maybe breaking our core/server into multiple pages would already help: https://webpack.js.org/concepts/entry-points/#multi-page-application

@skjnldsv
Copy link
Member

@ChristophWurst yes, it's planned! :)

If we'd make them all small independant apps. We'd get a real complex build script I guess.

I disagree. Having each app their own script is far better I think.
And with our drone build check it won't be difficult to ensure all of our compiled script are always up to date (if that's what you meant by a complex build script) :)

@ChristophWurst
Copy link
Member

I disagree. Having each app their own script is far better I think.
And with our drone build check it won't be difficult to ensure all of our compiled script are always up to date (if that's what you meant by a complex build script) :)

But separate apps will also mean we have to maintain all the dependencies individually. That's a lot of work, even when using dependabot.

@skjnldsv
Copy link
Member

@ChristophWurst yes it will be. especially when our components will have their own repo as well :)

@rullzer
Copy link
Member Author

rullzer commented Jun 28, 2018

@skjnldsv that would of course result in a massive amount of js. Like the contacts menu now is <14 kb. But if everything is bundled it would become at least 100kb.

There is still a lot of old jquery/handlebars to get rid of.

Like again let me point to the example with the authtokens: https://github.com/nextcloud/server/blob/master/settings/js/authtoken_view.js

That is in the settings apps. But how will we include this properly? Do we then add another small vue app inside the settings app?

@skjnldsv
Copy link
Member

But if everything is bundled it would become at least 100kb.

Why would it be bigger? Vue is supposed to make our code simpler :)
As long as we don't ship unwanted massive libraries with it, I don't see why we cant optimise it?

That is in the settings apps. But how will we include this properly? Do we then add another small vue app inside the settings app?

The main settings ≠ users+apps vue app.
Just create the authtoken vue app and load it into the #security like we do w/ handlebar?

@juliusknorr
Copy link
Member

Just create the authtoken vue app and load it into the #security like we do w/ handlebar?

I think we should at least have a common vue app for all the settings that are part of the core settings/. That could basically also use the existing vue base in settings/src/, we just need to make sure to just mount it on the content if it is not for the apps/user settings.

@skjnldsv
Copy link
Member

@juliushaertl I disagree, the 'plugin' system we have for every app to register their component via php is very efficient. If we want to migrate parts of this, we can create a new app for the settings and add route for full views like the user settings, but other way, I think we should keep thing separate.

I prefer not having one big app for every settings and multiple independant modules that are easier to maintain and can't be broken by another issue :)

@juliusknorr
Copy link
Member

I disagree, the 'plugin' system we have for every app to register their component via php is very efficient. If we want to migrate parts of this, we can create a new app for the settings and add route for full views like the user settings, but other way, I think we should keep thing separate.

Hm, good point, that would also break the possibility to easily add separate setting sections to one settings category.

@rullzer
Copy link
Member Author

rullzer commented Jun 28, 2018

Why would it be bigger? Vue is supposed to make our code simpler :)
As long as we don't ship unwanted massive libraries with it, I don't see why we cant optimise it?

ah well. So because we ship all libs for each vue app. Don't get me wrong I love the isolation. But right now vue does create large files. Like https://github.com/nextcloud/server/blob/master/apps/oauth2/js/oauth2.js for the oauth is pretty big compared to https://github.com/nextcloud/server/blob/7b8063a2424ea9f9d87493a23074b62afdd57854/apps/oauth2/js/setting-admin.js

Sure I moved some stuff to js that was in php before. But it is mainly from shipping all the libs.

I can easily imagine if we do this on the main page as well. And load the contactmenu, sharing sidebar, comments sidebar, version sidebar, files app etc. It can count up quickly in the amount of JS we will load.

@juliusknorr
Copy link
Member

For the main page we could maybe build a core vue app and use v-pre for the content, so other vue apps can just put their stuff in their without interfering with the contactsmenu, etc. Not sure if that would work like that, @skjnldsv ?

@skjnldsv
Copy link
Member

@rullzer then let's not compile the vue lib in it and only load it once for the whole site?
oauth2 would be 13KB only then.

Nonetheless, you forgot that vue also includes the templates. And if we want to be fair, removing all of our jQuery mess would probably lighten our load as well :)

For the main page we could maybe build a core vue app and use v-pre for the content, so other vue apps can just put their stuff in their without interfering with the contactsmenu, etc. Not sure if that would work like that, @skjnldsv ?

We could, but I don't like having a js injecting uncompiled html element. I mean, what's the point then?
Let's not overly complicate things :(

@juliusknorr
Copy link
Member

then let's not compile the vue lib in it and only load it once for the whole site?
oauth2 would be 13KB only then.

So we ship a global vue version for the vue apps included in the server repo and all other apps should basically bundle their own vue into their apps? Makes sense to me. 👍

@skjnldsv
Copy link
Member

Obviously this could be harder as this would require to always have the same vue version for ALL of our apps. If we get a major update, we'll have to update all of our apps :)

@rullzer
Copy link
Member Author

rullzer commented Jun 28, 2018

yeah lets for now first go with small apps even if they are bigger. In theory we could tell the browser anyways to cache this for a looooong time.

I actually like the '1 small app 1 function' kind of thing. Makes it simpler!

@rullzer
Copy link
Member Author

rullzer commented Jun 28, 2018

Of course this still would need some decision how to store it then. Like where do we put the authtoken code if we want to migrate that to vue?

@skjnldsv
Copy link
Member

skjnldsv commented Oct 1, 2018

Just as a reference, with dynamic import, webpack will only load the import it needs instead of the whole bundle. :)
See #11484

@rullzer
Copy link
Member Author

rullzer commented Nov 5, 2018

This is fixed by @ChristophWurst with his multiple entrypoints thingy

@rullzer rullzer closed this as completed Nov 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of enhancement feature: vue apps
Projects
None yet
Development

No branches or pull requests

5 participants