@diosmosis opened this Pull Request on August 31st 2021 Member

Description:

This PR introduces Vue 3 + all the tools required to build and use Vue 3 components within the context of Matomo.

Documentation on the Vue build system integration can be seen here: https://github.com/matomo-org/developer-documentation/pull/541 (as well as some documentation on Matomo's asset pipeline). It is recommended to read this new documentation first to understand how the integration is achieved.

To see how vue components are coded, take a look at the ExampleVue plugin in this PR. Or take a look at the proof of concept conversion of the activity indicator directive in CoreHome.

Getting started with vue locally

To test this PR out or just code with Vue, you should just need to checkout this PR's branch and run npm install. After that the vue:build command should work. If not, you may need to run npm install -g <a class='mention' href='https://github.com/vue'>@vue</a>/cli (but it shouldn't be necessary I think). Note: I am using the latest major version of node locally, v16.7.0.

Notes:

  • npm updated my package-lock.json dependencies everywhere for some reason. I've kept the updates for now.
  • This PR uses eslint instead of tslint. tslint appears to be deprecated in the Vue CLI tool, and eslint is generally a more flexible tool.
  • Vue 3 does not support IE11. So with this PR we dropping support for IE11 (at least for new Vue related code). This isn't really possible to work around. The compiled Vue JavaScript library distributed on npm directly uses some ES features (like const/let and JS proxies) that will not work in IE11, so even if we compile our JavaScript to work with IE11, Vue will not load properly.
  • There is a demonstration here in the ExampleVue plugin for Vue's async component feature. It should be noted that this feature does not load templates via AJAX like angularjs. It loades the entire component's JavaScript file over the network dynamically. This is sort of how Angular does it via routing. A description for how exactly this works is in the linked documentation PR.
  • Merged asset sizes for 4.x-dev + this branch are listed below. These metrics will be included in every [Vue] PR so we can monitor the change in asset size over time. There is a new command in this PR that is used to compute it.
  • Originally I used the class component API but noticed when reading through Vue docs and rfcs for 3.0, it doesn't appear to be as supported as the composition API. So I switched to the composition API. We could use the class component API, but would have to include extra code and we'd still use the composition API for some things in the adapter layer.

AngularJS/Vue communication & backwards compatibility

Every new Vue component has an angularjs directive adapter that initializes it. This is done for backwards compatibility and as a formal layer for AngularJS to communicate with Vue.

Examples for this are in the ExampleVue and CoreHome plugins.

From AngularJS => Vue, we use watches on scopes to change data. Conversely from Vue => AngularJS, events within Vue will be used.

Vue code will never embed or integrate AngularJS code. This is how we keep the layers separate and slowly hollow out the AngularJS code.

Merged asset sizes

image

image

image

Review

@diosmosis commented on August 31st 2021 Member

There are some files that need to be updated, but it looks like UI tests generally pass. This may not be indicative of there being no issues, however, since the only converted directive so far is the activity-indicator, which is not shown in UI tests.

@tsteur commented on September 2nd 2021 Member

@diosmosis out of curiosity I see there's also some babel code in there. Is that needed for some specific reasons?

Could we maybe also add one example where we add a template file async or maybe is it already in there and I missed it?

@diosmosis commented on September 2nd 2021 Member

@tsteur

out of curiosity I see there's also some babel code in there. Is that needed for some specific reasons?

This is documented here: https://github.com/matomo-org/developer-documentation/pull/541/files#diff-974b15ac14576588ebe1764f6280e0504a9263ee94237a3c6dc38f02df98476fR57-R60

It's not strictly necessary, but it seems to be better supported by Vue CLI than typescript alone. The Vue CLI project creator will in fact warn and ask again if you only select TypeScript and not babel.

EDIT: If after reading this + the docs you have specific concerns, please let me know.

Could we maybe also add one example where we add a template file async or maybe is it already in there and I missed it?

Yes, the ExampleVue plugin's component does this: https://github.com/matomo-org/matomo/pull/17940/files#diff-e658f42d3195754b19386bf7a2b75ae474a42933d6e011e0064cff6881b427a4R3

And there are docs around this here: https://github.com/matomo-org/developer-documentation/pull/541/files#diff-974b15ac14576588ebe1764f6280e0504a9263ee94237a3c6dc38f02df98476fR100-R119

It is worth reading the docs to understand (also note that the template file is not loaded async, the entire component JavaScript is).

@diosmosis commented on September 2nd 2021 Member

@tsteur I'll go through and add some github comments for this PR since it's a big one. Needed to all be done at once unfortunately (as far as I could see).

@diosmosis commented on September 5th 2021 Member

also note that the template file is not loaded async, the entire component JavaScript is

Note: this is the Vue (and I guess React/Angular) way of splitting the finished asset up, but it might be possible for us to manually issue an ajax request for a Vue template and use that in the promise to defineAsyncComponent(). However, we'd be doing things our own way not the Vue way. (Just thought of this.)

@diosmosis commented on September 7th 2021 Member

@sgiehl

I had to manually enable the CoreVue plugin, do we need an update script to enable that plugin on update?

Yes, good point :+1:

And another side note: Using all those new npm libraries will make it impossible to develop using Windows as running a npm install will fail due to the symlinks it tries to create.

This is interesting, I haven't tried it on windows but I can give it a try. Did you check if it works through WSL or is it just failing through the command prompt?

@diosmosis commented on September 7th 2021 Member

@sgiehl I was able to npm install and run vue:build on WSL. But compilation failed for another reason (a relative import failed, and apparently there are many users on windows w/ a similar issue). It's possible docker will also work.

@tsteur can you provide an opinion on the importance of windows support in this workflow?

@sgiehl commented on September 8th 2021 Member

@diosmosis didn't try WSL. Having the code in a virtual box or docker container works, but as soon as the code is stored on NTFS file system (and maybe mounted into a vm), npm will fail.
npm often has problems on windows, in most cases due to symlinks or too long path names. As long as none of those stuff is required to run when running Matomo I guess it might be fine to ignore it and maybe mention it in the developer docs.

@diosmosis commented on September 12th 2021 Member

@sgiehl in that case it'll be a problem no matter what framework we use. I'll modify the guide as you suggested.

@diosmosis commented on September 12th 2021 Member

@sgiehl updated this PR to use the local script and not the .bin file, and the developer-documentation pr to mention windows. Can you take another look?

@sgiehl commented on September 14th 2021 Member

@diosmosis had another look. Might be good to merge in the latest changes from 4.x-dev, which should fix some of the UI test failures. Some other UI tests files might need an update though due to the changed plugin list. Also there are some other tests failing. Guess you need to add ExampleVue to Kernel\PluginList.
Should we also already mention vue in the changelog and maybe directly link the developer docs there?
From my point of view that would be good to merge once the tests are passing.
Not sure if maybe @tsteur or someone else might want to have a look as well first.

@diosmosis commented on September 14th 2021 Member

@sgiehl @tsteur mentioned he won't need to look, however this won't be merged until after 4.5.0 is released so there is one release in between dropping support for IE11. I'll look at the tests.

EDIT: Changed the milestone to 4.6.

@github-actions[bot] commented on September 23rd 2021 Contributor

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

Powered by GitHub Issue Mirror