nextcloud / notes

✎ Distraction-free notes and writing
https://apps.nextcloud.com/apps/notes
GNU Affero General Public License v3.0
614 stars 134 forks source link

Move away from AngularJS, ideally to Vue.js (standard for Nextcloud apps) #241

Closed stefan-niedermann closed 5 years ago

stefan-niedermann commented 6 years ago

This might require some refactoring, but AngularJS is nearly its end of life.

Another benefit could be to motivate some new contributors to help code on the server app (including me).

I'd suggest to setup a new project using the angular cli and try to move everything that's possible.

korelstar commented 5 years ago

I totally agree that we have to replace the old AngularJS framework with something new. We definitely have to do something.

However, I did some research on this topic. It appears that most Nextcloud apps plan/work on switching or already switched to VueJS:

Already done:

Active work:

Planned:

And this is just a selection of most important apps.

Before, there was a longer discussion about a suitable framework/library:

Meanwhile, there is a project which develops specific components for a convenient integration of Vue in Nextcloud: https://github.com/nextcloud/nextcloud-vue

Considering this all, I think it would be best if the notes app switches to VueJS, too. Please note, this this is not a discussion about the best framework/library. It's more about how to integrate a Nextcloud app in it's ecosystem and consolidate the app infrastructure.

So, let's do it! Let's switch to VueJS!

Any help is very appreciated :-)

stefan-niedermann commented 5 years ago

Summoning Vue JS maestro @skjnldsv for some input.

Maybe we can migrate some components step by step or migrate the whole app at once? Maybe you have even some time to create some kind of roadmap/plan for a possible migration and some code input? :)

skjnldsv commented 5 years ago

Hey there :grin: :wave: Migrating step by step is hard regarding the big differences there is between angular and vue. Though migrating to vuejs is relatively quick, especially with the components on https://github.com/nextcloud/nextcloud-vue

The migration thoughts plan I used was:

  1. define features
  2. define current bottlenecks and see with features what fixes you could do
  3. plan data structure (if vuex for example)
  4. plan app structure (should be already 80% done with our standards, see docs)
  5. start migration
  6. make sure you're feature complete

It allowed us to avoid the same mistakes we did before and fix a crazy amount of issues. Planning the data structure should also include some perf tests if you had some bottlenecks here so you can find the best solution to the js perfs issues you had.

No idea how complicated the notes is, but I'll assume this is just a markdown editor, so I'm pretty sure you can already find some libs for that. https://github.com/vuejs/awesome-vue#markdown After defining this, what's left is just to fetch the data, calculate the navigation menu and use the official nextcloud appnavigation component, use vue-router to properly select the wanted note, display it on the content and open the editor? :)

For an example app you can check https://github.com/skjnldsv/vueexample. This is a simple one we used on some workshop I did with @juliushaertl ;)

if you have questions, shoot! :fire:

korelstar commented 5 years ago

Thanks for your overview, @skjnldsv :-) I will have a look at the example app, that should be helpful.

korelstar commented 5 years ago

Hey @skjnldsv,

there are so many dependecies to other JS libs in your example app. Are they all really required?

The demo app is doing nothing, but I get a warning from webpack, that the resulting JS is too large (packed, it's about half a mega byte).

I'm a friend of minimalism regarding dependencies. Can we kick something out?

skjnldsv commented 5 years ago

Yes, you can kick some! The app doesn't do anything, it's just a bad example of what you can do with the nextcloud components.

{
        "@babel/polyfill": "^7.0.0",    // NOT NEEDED
        "nextcloud-vue": "^0.4.2",
        "uuid": "^3.3.2",           // NOT NEEDED
        "vue": "^2.5.21",
        "vue-click-outside": "^1.0.7",  // NOT NEEDED
        "vue-router": "^3.0.1",
        "vuex": "^3.0.1",
        "vuex-router-sync": "^5.0.0"
}

For the rest since i'm sure you'll use vuex, you will still need them. The dev dependencies are, well, just devs ones, so it doesn't really matter.

For the vue components, one of our issue currently is that you can only import the full bundle in webpack. We'll probably fix it in a near future, so don't worry about this! :)

korelstar commented 5 years ago

Thanks, but that didn't change the resulting file size. I suppose that those libraries are required by other dependencies anyway.

However, I executed webpack-bundle-analyzer on the example app. The result is, that the biggest part is coming from nextcloud-vue (290KB of 466KB):

grafik

Hence, I think It would be really nice, if this dependency can shrink. :smiley:

skjnldsv commented 5 years ago

For the vue components, one of our issue currently is that you can only import the full bundle in webpack. We'll probably fix it in a near future, so don't worry about this! :)

Like I said, you're 100% right :see_no_evil: Not really have much time lately, so if you want to dig in it, feel free! It would be highly appreciated! :wink:

I removed the vue compiler as well! This is not needed and make us win a few KB as well ;)

korelstar commented 5 years ago

Yeah, I would like to, but webpack is totally new for me. Let's see what I can contribute in a few months ...

nicolad commented 5 years ago

Thanks, but that didn't change the resulting file size. I suppose that those libraries are required by other dependencies anyway.

However, I executed webpack-bundle-analyzer on the example app. The result is, that the biggest part is coming from nextcloud-vue (290KB of 466KB):

grafik

Hence, I think It would be really nice, if this dependency can shrink.

Good point, I think that it would be nice if nextcloud-vue would expose an interface from which it will be possible to cherry-pick just those components that are only used, not entire library. I've seen this being done via monorepo, using tools like https://github.com/lerna/lerna Here are some example of such package: https://www.npmjs.com/package/@wordpress/a11y https://www.npmjs.com/package/lodash.chunk Is this what you expect from nextcloud-vue ?

skjnldsv commented 5 years ago

Good point, I think that it would be nice if nextcloud-vue would expose an interface from which it will be possible to cherry-pick just those components that are only used, not entire library.

Yes, this is called tree shaking apparently: https://webpack.js.org/guides/tree-shaking/ No need for mono repo, it's just a matter of having either a proper webpack conf so that any further project using webpack also understand the library, or split each component into individual js files as well as a full one. Like https://github.com/vuematerial/vue-material#installation-and-usage import { MdButton, MdContent, MdTabs } from 'vue-material/dist/components'

Ideally, we should aim to that ;)

korelstar commented 5 years ago

I've created the branch vue and PR #290 in order to track the work on implementing. Any help is welcome. :smiley: