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] Introduce Vue + Workflow commands #17940

Merged
merged 44 commits into from Sep 28, 2021
Merged

Conversation

diosmosis
Copy link
Member

@diosmosis diosmosis commented Aug 31, 2021

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: matomo-org/developer-documentation#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 @vue/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 diosmosis added this to the 4.5.0 milestone Aug 31, 2021
@diosmosis diosmosis marked this pull request as draft August 31, 2021 03:00
@diosmosis diosmosis changed the title [Vue] initial integration [Vue] Introduce Vue + Workflow commands Aug 31, 2021
@diosmosis
Copy link
Member Author

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

@diosmosis
Copy link
Member Author

@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
Copy link
Member

sgiehl commented Sep 14, 2021

@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
Copy link
Member Author

diosmosis commented Sep 14, 2021

@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.

@diosmosis diosmosis modified the milestones: 4.5.0, 4.6.0 Sep 14, 2021
core/Updates/4.5.0-b2.php Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

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

@github-actions github-actions bot added the Stale The label used by the Close Stale Issues action label Sep 23, 2021
@diosmosis diosmosis added Do not close PRs with this label won't be marked as stale by the Close Stale Issues action and removed Stale The label used by the Close Stale Issues action Needs Review PRs that need a code review labels Sep 23, 2021
@sgiehl
Copy link
Member

sgiehl commented Sep 27, 2021

@diosmosis would you mind resolving the conflicts, so we can merge this soonish?

@diosmosis
Copy link
Member Author

@sgiehl updated

Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me now 👍

@sgiehl sgiehl merged commit 2ac2bc1 into 4.x-dev Sep 28, 2021
@sgiehl sgiehl deleted the vue-initial-integration branch September 28, 2021 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Do not close PRs with this label won't be marked as stale by the Close Stale Issues action
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants