Page MenuHomePhabricator

Add Vue.js v2 runtime-only Core or Vector ResourceLoader module for new Vue.js search
Closed, ResolvedPublic3 Estimated Story Points

Description

As concluded in T249725, Vue.js v2 will be used for search. The project could either:

  1. Depend on the Vue.js compiler + runtime ResourceLoader module currently available in Core.
  2. Depend on a new Vue.js runtime-only ResourceLoader module in Core.
  3. Build the Vue.js runtime into the components entry.
  4. Expose a new Vue.js runtime-only ResourceLoader module in Vector.
  5. Something else.

According to the documentation, shipping the Vue.js runtime without the compiler is about 30% lighter. There is also a CPU performance savings for every pageview that scales with the number of components shipped. Since the pilot project is risky by nature and user interactivity makes it especially timing sensitive, minimizing the payload as much as possible is preferable and the obvious choice seems to be #2.

That is, expose a new Vue.js v2 runtime-only ResourceLoader module in Core with no other dependencies.

Note: WMDE's Vue templates are pre-compiled so they may wish to leverage the new dependency as well.

New acceptance criteria

  • A new search only ResourceLoader module is added to Vector. Initially, this will only contain the Vue.js minified, production runtime. Eventually, it will contain WVUI and any new search experience code needed.
  • T251974 is revised to document migration requirements for Vector.

Old acceptance criteria

  • The Vue.js minified, production runtime is exposed as a ResourceLoader module with no other dependencies.
  • The Vue.js minified, production compiler module actually depends on the runtime ResourceLoader module so that 1) it never needs to be loaded twice 2) it must be kept in sync with the runtime.
  • The Vue.js development runtime is exposed as a ResourceLoader module. It is probably ok if this shares the same module as the development compiler + runtime.
  • Naming and versioning is considered. I'm guessing whatever Roan decides is fine but WMDE contacts are CC'd on this ticket. It would be neat if the names corresponded to the official distributables.

References

Event Timeline

@holger.knust, are you still planning to work on this task or should we ask @Catrope? Either way works for me so long as we have it.

@Niedzielski not ready to work on it immediately so if you want to ask @Catrope that works for me.

Change 603458 had a related patch set uploaded (by Holger Knust; owner: Holger Knust):
[mediawiki/core@master] resourceloader: Add Vue 2.6.11 runtime-only library

https://gerrit.wikimedia.org/r/603458

Niedzielski added a subscriber: ovasileva.

@ovasileva, I'm adding this to the Web board for tracking and visibility purposes. Holger is working on this task.

@holger.knust, process note: I've moved this task to the code review column on the Web board and popped your name off to indicate that a reviewer is wanted.

Thanks, @Niedzielski. I added the runtime-only library as a separate module so that developers have a choice of selecting either 'vue.runtime' for just the runtime or 'vue' for runtime and compiler as before. If we decide to keep the two options we may need to tweak the acceptance criteria a bit.

The current patch set does not include versioning. We may want to add the major version number to the module name, i.e. 'vue2' and 'vue2.runtime', and document that the latest stable release of the major version will be used in production. I think the versioning process will require a little more work/thought to make it less dependent (maybe independent altogether if possible) on someone updating the libraries in Core. By looking through Vue's release history (see below), it appears the Vue project goes through bursts which could make it hard to keep everything in sync.

Thank you, @holger.knust!

'vue.runtime' for just the runtime or 'vue' for runtime and compiler as before

Makes sense. I think that ideally the vue module would depend on vue.runtime module. The reason is that some pages may want the compiler and they should be allowed to include it by adding a non-conflicting 30KB JS instead of 90KB of partly redundant JS. Unfortunately, I don't think the compiler is pulled out in Vue 2, only Vue 3, so I think that should be a future enhancement that's out of scope for this ticket.

The current patch set does not include versioning.

Good note. I think versioning is out of scope for this ticket but know that it has come up several times previously and @Catrope is aware.

The current patch set does not include versioning.

Good note. I think versioning is out of scope for this ticket but know that it has come up several times previously and @Catrope is aware.

That is my concern as well. Seeing how much trouble and disruption was caused by the recent HTML - JS+CSS desynchronization, I think it's worth discussing what it takes to link specific versions of a bundle from HTML. Afaict the assets not being generated by the time the HTML is first served is the reason why this is not done by RL, but you might know better.

ovasileva triaged this task as Medium priority.Jun 8 2020, 6:34 PM
ovasileva set the point value for this task to 3.Jun 15 2020, 5:26 PM

@Demian, thank you. I agree that versioning is very important and related but separating the Vue.js runtime using the existing patterns shouldn't need to take on that hard work too. I would like to keep this task focused on the runtime and keep the Vue.js versioning conversations within T251974.

@Catrope, I owed you a follow-up from our Slack conversation! Here's the summary of our shared understanding so far as I know:

  • This runtime-only build is needed by search.
  • Vue.js v2 itself does not provide a compiler build and Wikimedia does not want to unnecessarily create and maintain a custom build.
  • Vue.js v3 provides the compiler separately but it's not out yet.

Unless you have a better approach, it's my recommendation that during Vue.js v2 usage, a limitation that entries must either use the the runtime module or the compiler+runtime module, not both be made. Loading the compiler+runtime instance over the runtime instance or vice-versa will overwrite the window.Vue global. This appears to work properly but serves redundant bytes so it should be avoided.

However, the following asynchronous scenarios are unsupported as any window.Vue global changes would be lost:

  • If the compiler+runtime module is already loaded, simply do not load the runtime. You already have everything.
  • If the runtime is loaded, you cannot load the runtime+compiler dynamically.

If the latter case is important, simply load the compiler+runtime instead of the runtime but the main namespace does not need it.

These limitations will be lifted when v3 is available and the compiler and runtime can be split properly. Ideally, this means the runtime can live in Core even during v2.

@Catrope, this is a friendly reminder to follow up on the patch. I know you're very busy. I'll stop talking now :]

Hi, I see RL was tagged here but I'm sure what the need is from RL for this task. What is the question?

Development of modules for different areas of core or extensions are generally tracked under the component they belong to (only mw.loader etc is tracked under RL).

Module registration overhead might warrant a perf review (which is implicitly also tagged), but I believe that is mostly captured in T249306.

As for the compiler... if the search widget will be entirely made up of pre-compiled standalone components from the component libary and/or written in a plain form in a MW repo that requires no compiling, then shipping the runtime only could work. I did not think of that previously as an option, but looks quite interesting indeed. Is that the case?

If not, then given the initial pilot for search will not involve a build step in production, it seems the only option is to ship the default Vue.js payload with the additional uncompiled Vue code. That's indeed not great for performance, and this was pointed out during the FAWG discussions as a drawback, but that's the compromise we made for the initial launch.

Development of modules for different areas of core or extensions are generally tracked under the component they belong to (only mw.loader etc is tracked under RL).

My mistake! I've removed the MediaWiki-ResourceLoader tag.

if the search widget will be entirely made up of pre-compiled standalone components from the component libary and/or written in a plain form in a MW repo that requires no compiling, then shipping the runtime only could work. I did not think of that previously as an option, but looks quite interesting indeed. Is that the case?

Yes, the search widget will be entirely pre-compiled.

If not, then given the initial pilot for search will not involve a build step in production,

This is still being discussed. Regardless, a couple other options exist but they're not ideal: 1) compile the Vue.js runtime into the WVUI library 2) add the Vue.js runtime as a distinct Vector ResourceLoader module (or some place not Core).

@Catrope, ping! For purposes of scheduling, do you have an idea when you might be able to fit this patch into your review queue? Thank you!

@Catrope, ping! For purposes of scheduling, do you have an idea when you might be able to fit this patch into your review queue? Thank you!

So sorry for failing to follow up on this, and for initially missing your ping!

@Catrope, I owed you a follow-up from our Slack conversation! Here's the summary of our shared understanding so far as I know:

  • This runtime-only build is needed by search.

I wouldn't quite characterize it as "needed", necessarily, but it's a desirable performance improvement over the alternative (cc @Krinkle for the Performance-Team 's take on this)

  • Vue.js v2 itself does not provide a compiler build and Wikimedia does not want to unnecessarily create and maintain a custom build.
  • Vue.js v3 provides the compiler separately but it's not out yet.

Unless you have a better approach, it's my recommendation that during Vue.js v2 usage, a limitation that entries must either use the the runtime module or the compiler+runtime module, not both be made. Loading the compiler+runtime instance over the runtime instance or vice-versa will overwrite the window.Vue global. This appears to work properly but serves redundant bytes so it should be avoided.

However, the following asynchronous scenarios are unsupported as any window.Vue global changes would be lost:

You say "this appears to work properly", so what global changes to window.Vue would actually be lost? Are we talking about plugins that have been loaded or something like that? I agree this is best avoided, but would it actually break in reasonable use cases?

  • If the compiler+runtime module is already loaded, simply do not load the runtime. You already have everything.

This is actually pretty easy to implement using a skipFunction, and should probably be added to the vue.runtime patch. I'll leave a comment on Gerrit.

  • If the runtime is loaded, you cannot load the runtime+compiler dynamically.

This is a harder one, because...

If the latter case is important, simply load the compiler+runtime instead of the runtime but the main namespace does not need it.

one (typically) does not "simply" determine whether the compiler+runtime is going to be loaded, especially once we convert something like the Echo dropdown to Vue: a feature that needs the compiler, is almost never loaded initially, but can be lazy-loaded (triggered by user interaction) on almost any page. If, similarly, the search feature also uses lazy loading and is present on every page, then we end up in the weird situation where if the user first opens the Echo dropdown then later interacts with search, everything is fine because the Echo lazy-loading has already loaded the runtime+compiler, but if they do those things in reverse order then things break because the search has loaded the runtime-only module and Echo now can't load the compiler that it needs. In order to avoid this situation we'd need a (relatively) magical way to load the runtime+compiler module even in some cases where the runtime-only module was requested, because we know there's something else on the page that could try to lazy-load compiler-dependent code later.

These limitations will be lifted when v3 is available and the compiler and runtime can be split properly. Ideally, this means the runtime can live in Core even during v2.

Since we'll be able to solve this problem properly when v3 arrives, I wonder if it wouldn't make more sense to avoid building more complexity and instead eat the performance cost for now.

I agree with Roan that it might not be worth the engineering cost of making this work with v2 with v3 around the corner. In terms of perf cost, this payload was already approved for the initial pilot project to use Vue in the on-demand search experience.

Having said that if you can spare the time and can find a way to make it work with v2 without functional problems or split-brain/lost global state, I'd be fine with that even it (temporarily) costs one additional module. In particular thinking about Search being page-agnostic which means it needs to play along on special pages where CX co-exists, and with Echo which is similarly page-agnostic.

I don't know if it's possible to avoid entirely the need for these two to be active on the same page, but if you think that's feasible, then I'd also be fine with not having a strategy for that theoretical context - so long as there is a consistent fatal error produced (either client or server-side) from that presumed impossible condition so that we find out if and when that condition turns out to not be impossible.

Thank you both, @Catrope and @Krinkle. So, how do you think we should move forward? I think I understand the concerns but not the priorities. This task is slowing down the progress of the Vue.js search project so I'd like to make sure we're not going around in circles.

As an invested member of the search project, I am quite concerned about the actual delay of loading the compiler in addition to everything else. I want the project have the best chance at success and shipping the compiler seems like a significant bandwidth consumer and also a highly unusual Vue production configuration. Don't forget that in addition to the hefty Vue runtime itself that search must block on, we still have to load all of the ULS input tools and its jQuery dependencies, and the new search feature code. All those dependencies aren't even requested until search focus. I think that adding the compiler to this payload is a big risk to search user engagement.

I don't know if it's possible to avoid entirely the need for these two to be active on the same page, but if you think that's feasible, then I'd also be fine with not having a strategy for that theoretical context - so long as there is a consistent fatal error produced (either client or server-side) from that presumed impossible condition so that we find out if and when that condition turns out to not be impossible.

So far as I can tell, we don't need to actually support this runtime-only and compiler+runtime configuration if we don't want to for Vue 2. For Vue.js search, our target wikis don't include Commons (MachineVision) or Wikidata, NearbyPages isn't deployed anywhere yet, and ContentTranslation is compiled (/cc @santhosh). I don't think the hypothetical scenarios posited will be encountered before we migrate to Vue 3 in which case this will not be an issue at all.

For the duration of the search project, I would like to have this second runtime-only module available. As @Krinkle alluded to, we don't have bandwidth to develop custom Wikimedia-only solutions for possible but unlikely scenarios. If those scenarios are needed for Vue 2, I think the onus is on those teams to build the custom solutions. If it's a stability issue for Core platform to have the new module there, we can bundle Vue into the library itself but Core seems like a better place to me.

Adding Platform Engineering as I'm unsure if I should have tagged them when removing mediawiki-resourceloader.

We discussed how to move forward with this task today in our weekly Web meeting. The options appear to be:

  • Option 1: provide in Core. This seems like it will take some effort until Vue 3 is available which properly separates the compiler.
  • Option 2: bundle in Vector search ResourceLoader module entry. This adds the runtime dependency directly with the other search code. The pro and con is that it is not exposed.
  • Option 3: bundle into WVUI entry. This approach can benefit maximally from compilation.
  • Option 4: add a dedicated Vue 2 runtime Resourceloader module. The pro and con is that is exposed. It would be possible for external code to depend on it.
  • Option 5: wait for Vue 3. This may not be possible given project and Vue release timelines.

We think that pursuing option 2 (or that failing, 3 or 4) for Vue.js search is not optimal but is the most practical, safe, and future-facing. This means that Vector will have to migrate to the Vue 3 Core module once it is available. I've added an acceptance criteria to revise the pre-planning task with detail on Vector's usage.

As of this week, Vue3 is now in the release candidate stage.

Since it will still be a few months (?) before the new search tool is deployed, it might make sense to just "embrace the future" and add a set of Vue3 modules to core: one for the runtime, and one for the compiler. Since no code is currently relying on these modules, we could just make sure we keep them up to date with any upstream changes that happen between release candidate and final release of Vue 3. Would that simplify things for anyone?

Also, there are currently so few Vue-based things in production that it would be easy enough to migrate existing projects to a "Vue2" module and let the new modules be the definitive ones going forward.

In general, I support the idea of having separate resource modules for runtime and compiler, so that developers working with Vue can limit their dependencies to the code they actually need.

Change 616311 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/skins/Vector@feat/search] [WIP][build] Integrate Wikimedia Vue User Interface components

https://gerrit.wikimedia.org/r/616311

Thank you! The Vue 3 security review is tracked in T257734. Its availability in Core would give us more flexibility in how we move forward but it's unclear if migration to v3 will in scope or not for search.

If a Vue 2 runtime is made available in Core distinct from the compiler, the intent of this task originally, that will open up more options. From the discussion above, it sounded like there may be reasons not to so we're actively working to bake this directly into Vector using standard ES modules currently.

Change 617587 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/skins/Vector@master] [WIP][build] Integrate Wikimedia Vue User Interface components

https://gerrit.wikimedia.org/r/617587

Change 617587 abandoned by Niedzielski:
[mediawiki/skins/Vector@master] [WIP][build] Integrate Wikimedia Vue User Interface components

Reason:

https://gerrit.wikimedia.org/r/617587

Change 616311 merged by jenkins-bot:
[mediawiki/skins/Vector@feat/search] [build] Integrate Wikimedia Vue User Interface components

https://gerrit.wikimedia.org/r/616311

Niedzielski renamed this task from Add Vue.js v2 runtime-only Core ResourceLoader module for new Vue.js search to Add Vue.js v2 runtime-only Core or Vector ResourceLoader module for new Vue.js search.Aug 5 2020, 11:13 PM

T251974 is revised to document migration requirements for Vector.

As you point out in T252348#6306686, having the bundle inside Vector means that on certain page views, as we adopt Vue further, Vue will be loaded twice. Although as you also point out the wikis being targetted right now do not create that problem.

Despite this, I would be worried that when we make this move we would be slowing down the adoption of Vue elsewhere and we would be blocking further rollout of search meaning we definitely need to support 2 searches in production. Limbo states are not a good place to be from a code maintenance point of view.

It's my understanding when we have Vue 3 in core ( T251974) this problem goes away, but also ideally we don't want to block on that.

I think this decision is definitely something we'll want to re-review before deployment. Is there a deployment task or a generic case study review task that we can record a need to review this or should I record it in T251974 ?

If you want the review to block deployment, it should go in T249297. If the review is post-deployment, it should go in T251974.

Thanks I've added doing a risk assessment to T249297.