Page MenuHomePhabricator

[Spike 12.75 hrs] Where does the application-specific Vue.js search code live and what impact does this decision have on the long term?
Closed, ResolvedPublicSpike

Description

Question: Practically, are we planning to build the new search in core or Vector?
Outcome:https://phabricator.wikimedia.org/T244392 is updated with the outcome of this discussion

background

Currently the search widget is in core and is loaded by a core module based on the existence of an element with a given ID in the HTML. If we change that ID we break gadgets so we can’t do that.
https://github.com/wikimedia/mediawiki/blob/bbb7beab650bf490c9bf1d884b4a949ced1ae2fc/resources/src/mediawiki.page.ready/ready.js#L87

Minerva is the only skin that turns this off and the only reason it is able to do this is it doesn’t load mediawiki.page.ready (which also turns off sortable tables and collapsible elements) which is not an option for Vector. See open bug T111565 for more context.

If we’re planning to build the new search in Vector I guess we’ll need to make some architectural changes in core to support that. Although I can see a lot of benefits of doing this in core too

Questions to answer

  1. Should the new search component be built in core, Vector or a standalone repo?
  2. What changes in core are needed to support Vector (and in future Minerva) changing the search implementation?
  3. What does the roadmap look like for removing the existing code? Should we plan to make the new search component apply to all skins or will we retain the old code? Where will that code live? Who will maintain it?

Signoff criteria

  • Communicate recommendations from this task to FAWG, WMDE (@WMDE-leszek and @Pablo-WMDE may be good contacts), and other stakeholders and document results

Related Objects

Event Timeline

Restricted Application added subscribers: Masumrezarock100, Aklapper. · View Herald TranscriptMar 31 2020, 8:39 PM
Jdlrobson renamed this task from [Spike] Where does the Vue.js code live and what impact does this decision hae on the long term? to [Spike] Where does the Vue.js code live and what impact does this decision have on the long term?.Mar 31 2020, 8:39 PM
Jdlrobson added a subscriber: ovasileva.
ovasileva triaged this task as High priority.Apr 1 2020, 9:01 AM
Restricted Application changed the subtype of this task from "Task" to "Spike". · View Herald TranscriptApr 3 2020, 3:40 AM
Niedzielski renamed this task from [Spike] Where does the Vue.js code live and what impact does this decision have on the long term? to [Spike] Where does the Vue.js search code live and what impact does this decision have on the long term?.Apr 5 2020, 4:22 PM
ovasileva renamed this task from [Spike] Where does the Vue.js search code live and what impact does this decision have on the long term? to [Spike 13hrs] Where does the Vue.js search code live and what impact does this decision have on the long term?.Apr 6 2020, 5:32 PM
ovasileva updated the task description. (Show Details)
ovasileva renamed this task from [Spike 13hrs] Where does the Vue.js search code live and what impact does this decision have on the long term? to [Spike 12.75 hrs] Where does the Vue.js search code live and what impact does this decision have on the long term?.Apr 6 2020, 5:34 PM
Niedzielski updated the task description. (Show Details)Apr 6 2020, 5:44 PM
Niedzielski added subscribers: Pablo-WMDE, WMDE-leszek.
nray added subscribers: Jdrewniak, Volker_E, phuedx, nray.EditedApr 7 2020, 11:01 PM

@Jdrewniak @Volker_E @Jdlrobson @Niedzielski @phuedx and anyone else interested - Do any of you have a strong preference for where you think this component should live? If so, it would be awesome to get an initial pulse of that on this ticket now.

My initial thoughts are that I think for the short term and for the sake of the case study's purposes, I care more about how the component is architected rather than where it lives. If it's architected to be modular, reusable, and its concerns are separated well enough, it should be easy to move around with ease. For the long term and if the case study is a success, I think an open source npm component library would be awesome assuming that all of the existing ones don't meet our needs.

I know we've been calling this the "search" widget, but I think it would be wise to have at least two reusable components here at the high-level:

  1. Autocomplete Component - Responsible for reacting to typed input and displaying a list of suggestions with each keystroke. Perhaps most importantly, this component knows nothing about MediaWiki, the MediaWiki API, or how to retrieve the source of data. All of that is an implementation detail that should be left to the client. Instead, the data is passed in as a prop (e.g. might be a static list of data or a generic Promise that resolves to a list of data). Since it knows nothing about MediaWiki, there are no bounds placed on its reusability. One day, this might be part of a generic npm component library.
  2. MediaWiki Search Component - This would be composed of the AutoComplete Component and would be more tailored for how we do search at MediaWiki. Therefore, it would know how to interact with the MediaWiki API, know how to display thumbnails next to the autocomplete items, etc. Its reusability would be limited to the MediaWiki ecosystem (e.g. Vector Skin, Minerva Skin, etc). One day, this might be part of core (or maybe another npm library tailored to MediaWiki).

Do any of you have a strong preference for where you think this component should live? If so, it would be awesome to get an initial pulse of that on this ticket now.

I don't know if it's feasible, but I feel a strong leaning to have a dedicated repository that we publish to npm. In terms of integration I'd like to see us take the built and versioned output of that code and put it in either core or Vector.

I think with regards to the question of where the byproduct of that repo goes I'm open but I'd prefer the path of least resistance. I'm a little worried if we do it in Vector that we're going to need to move mountains in core. I'd love to see a POC showing how the existing search widget would be swapped out to help me make a decision with regards to that question.

I would prefer a separate repo because it enforces some objectives of the project including:

  • We dogfood library distribution via NPM. This means our code is more likely to be shareable.
  • We encourage development and production builds that work within and without MediaWiki.
  • We want a separate repo in the long-term.

That said, I'm flexible and wouldn't block on building in the repo unless there were other tradeoffs.


I know we've been calling this the "search" widget, but I think it would be wise to have at least two reusable components here at the high-level:

I think all of the components needed for search are likely to be reusable. Some examples include buttons, list items, lists, thumbnails, headers, and the search form itself.

Chiming in from the WMDE side (at the risk of missing the point b/c of lack of access to conversations that were had off of this phab ticket, and also at the risk of being captain obvious).

I see the term "component library" in this ticket here, but we made the experience that it is beneficial to clearly separate (mentally but consequently also in communication) between

  • the app(s) we build and which rules apply for them
  • a component library (shared between multiple apps) and which rules apply for it

For the latter I understood T249658: [Spike 12hr] Should shared components be compiled for Vue.js search? to be the relevant ticket.

The difference is not merely semantic but the Definition of Done for components to be added to the component library tends to be much more involved while work within individual apps, of different maturity, might benefit from being able to quickly iterate ideas. For the sake of a "Spike", focusing on this one app might offer much needed leg room in this category - and it could still inform or even bear components to be move to the shared library later on.

That said, we'd be happy to see a vue-based component library, following the WM design system, to come into existence and extend our willingness to (further) invest.

@Pablo-WMDE, thank you for your feedback! This is a very useful distinction to consider.

So, we have several existing component collections (some embedded in applications) including:

@Pablo-WMDE, judging by T240329 it looks like the library was made only recently. Is WMDE in the process of migrating common components from your apps to this library? I think there could be an opportunity for finding some common goals to co-develop this new library you have made that solves a lot of these infrastructure tasks (T249298). Perhaps, as you noted, this task can be about ensuring that the application infratstructure needed is available in Vector but all components "graduate" to the shared component library. I don't know if this is something that would be practical, or if there's a spirit of interest from WMDE, but as an individual contributor, codeveloping this component library sounds very appealing and I think we could have a lot of common ground.

Niedzielski renamed this task from [Spike 12.75 hrs] Where does the Vue.js search code live and what impact does this decision have on the long term? to [Spike 12.75 hrs] Where does the application-specific Vue.js search code live and what impact does this decision have on the long term?.Apr 9 2020, 4:25 PM

Per the above discussion and further comments on Slack, I've relabeled this task to focus on the "ripening" and application-specific stage and also signaled on the big epic my understanding of TechCom's requirements for component sharing which are relevant. T249840 tracks the common use case.

Good points here. Me and @Pablo-WMDE have exchanged some thoughts on this topic with @Niedzielski off-Phabricator, trying to summarize this here.

WMDE definitely sees value in having a shared base component library. On the conceptual level we see Wikidata specific components conceptually built on top of "base" Wikimedia one.
"Shared library" term is to be understood in the meaning as @Pablo-WMDE explained above.

For us it is important to underline that we see the vue implementation of the components only as a part of the "problem" here. The other, equally important and relevant, one is the Design side. This is also why at WMDE we see the vue implementation going hand in hand with the design work done by our Design colleagues (see Wikidata Design System). We believe that developer efforts are bound to fail, if the Design part is not on-board throughout the whole process. This way we ensure that we see UI elements the same way, and not just have devs play catch up with changes that Design might make.

We see creating and maintaining the "shared user experience" as a work that should be coordinated and executed consistently in both areas. It seems that getting alignment on the political level is the first step to take, to ensure all parties have the shared goal. This part is getting started but we're simply not there yet.

Re WMDE's simple vuejs component library - it is true that only recently, having created three separate _apps_ we've started recognizing components that would make sense to maintain centrally. Migrating common components has been relatively limited so far as we realized both that we should synchronize our efforts with the WMDE Design folks, and, more interestingly here, that probably it is a topic bigger than WMDE-only activity.

nray added a comment.Apr 16 2020, 10:09 PM

I think it would be great to cut down on the number of different search implementations [1] and have one shared standard solution which makes me lean towards building Vue search in core so that other projects can use it. I think it's going to have to be done incrementally rather than all at once though. For example, something like:

  1. Build new search for latest Vector. Latest Vector is the only client of this search at first.
  2. If above is successful, replace older search implementations with Vue search. For example and perhaps in a different order, replace legacy Vector search with Vue search, then replace Minerva search with Vue search, then replace Timeless search with Vue search (if desired), etc.

Currently the search widget is in core and is loaded by a core module based on the existence of an element with a given ID in the HTML. If we change that ID we break gadgets so we can’t do that.

@Jdlrobson Assuming an incremental approach outlined above is viable, do you think it would be a problem if we initially kept this ID for legacy Vector (and other projects that depend on mediawiki.searchSuggest), but had a different ID for the search box in latest Vector so that mediawiki.searchSuggest won't load and we could instead load Vue search? I understand that this would still break gadgets for latest Vector, but weren't we planning on this happening in latest Vector?

[1] https://www.mediawiki.org/wiki/User:JDrewniak_(WMF)/notes/Search_widgets_at_Wikimedia

@nray, I think inclusion within Core as an NPM package or even a submodule is a great idea (and only a slight pivot from yours) but I don't think we should house the component source and development there now or in the future. Massive +1 for dropping multiple search implementations too!

To add a little clarity to T249051#6038456, my strong preference is that all future users would eventually consume the components (search included) as an NPM package so that we can intelligently divide globally reusable components from the rest of the MediaWiki ecosystem. Designers, developers, newcomers, and everyone in between should be able to at least develop great components without having to standup a Vagrant instance and that's much harder to do within Core. For example, does Core even want Storybook? It's my understanding that Core has far greater platform level stability requirements and so development there is quite challenging and consequently slow.

In my view, Core would consume search and other components like any other user instead of being a special case or worse, an implicit dependency of all components. In my opinion, that would set us up for failure to meet the goals identified in T225572 and avoid the anti-patterns of T225453.

nray added a comment.EditedApr 16 2020, 11:48 PM

@Niedzielski Thank you, putting search in an NPM package and inclusion of that in core makes sense to me. What's most important to me is making it available to more projects than just Vector :)

I mentioned core in my most recent post because I still envision having components that have different levels of knowledge/dependence on MediaWiki [1]. For example, some components won't need to know anything about MediaWiki except to follow a design spec like a Button, Heading, and even an Autocomplete component. If we want a MediaWiki Search component to be a "drop-in" component that different projects can use with minimal fuss and configuration like what Isarra suggests [2], however, I think would require having some component that knows how to interact with a MediaWiki API. This could be done, for example, by wiring a more generic Autocomplete component so that it can do MediaWiki search. It was this wiring that I was proposing being in core. I was assuming the more generic Autocomplete component would live in an NPM library covered by T249840.

Having said that and for all the reasons that you conveyed, it might still be better to have ALL the components live in the same NPM library regardless of their coupling to any MediaWiki APIs. I'd be okay with that too!

[1] https://phabricator.wikimedia.org/T249051#6038241
[2] https://phabricator.wikimedia.org/T219590#6053057

[1]

Sorry, I totally forgot about that. I think I've got it now!

generic Autocomplete component so that it can do MediaWiki search

I would like to explore this idea more. We'll need to build a MediaWiki search client for sure, and that should be shared in some place, but I would love to be able to inject that as an interface dependency of a component (another interface implementation might for mock unit tests, for example). In the Popups code base, the gateway is determined at link hover time all the way up in src/index. I think that makes it easier to thread a new gateway through the system as needed. I certainly don't mean to say that the design will be easy though! :]

Change 591170 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/skins/Vector@master] POC Option #1: Load Vue Search By Setting 'wgUseVueSearch' Config

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

Change 591169 had a related patch set uploaded (by Nray; owner: Nick Ray):
[mediawiki/core@master] POC Option #1: Make the Search Widget to lazy load Configurable

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

Change 591170 abandoned by Nray:
POC Option #1: Load Vue Search By Setting 'wgUseVueSearch' Config

Reason:
Not mergeable just meant as a demo

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

Change 591169 abandoned by Nray:
POC Option #1: Make the Search Widget to lazy load Configurable

Reason:
Not mergeable just meant as a demo

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

Change 591171 had a related patch set uploaded (by Nray; owner: Nick Ray):
[mediawiki/core@master] POC Option #2: Modularize the Lazy Loading of mediawiki.searchSuggest

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

Change 591172 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/skins/Vector@master] POC Option #2: Replace mediawiki.searchSuggest with Vue Search Via Module

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

Change 591171 abandoned by Nray:
POC Option #2: Modularize the Lazy Loading of mediawiki.searchSuggest

Reason:
Not mergeable just meant as a demo

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

Change 591172 abandoned by Nray:
POC Option #2: Replace mediawiki.searchSuggest with Vue Search Via Module

Reason:
Not mergeable just meant as a demo

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

nray added a comment.EditedApr 20 2020, 11:10 PM

Should the new search component be built in core, Vector or a standalone repo?

Given there is clearly a demonstrated use case for having a shared standard Search widget in MediaWiki [1], and given that there are already multiple Vue component collections sprouting up already (see T249840), I think it makes sense to setup a centralized and standalone npm library repo for these components now. There are several reasons why developing these components in Core might be be disadvantageous as nicely summarized by @Niedzielski in T249051#6064443. The product from this npm library could be included in core so that multiple projects could use it.

What changes in core are needed to support Vector (and in future Minerva) changing the search implementation?

I think with regards to the question of where the byproduct of that repo goes I'm open but I'd prefer the path of least resistance. I'm a little worried if we do it in Vector that we're going to need to move mountains in core. I'd love to see a POC showing how the existing search widget would be swapped out to help me make a decision with regards to that question.

I think there are at least 3 options for this:

Option #1: Use the existing code that lazy loads search, but make the module it loads configurable

Core Patch: https://gerrit.wikimedia.org/r/591169
Vector Patch: https://gerrit.wikimedia.org/r/591170

Pros:

  • Simple to opt-in (just set wgUseVueSearch => true
  • Doesn't add more modules which would otherwise increase the payload of the resource loader startup module for all users.
  • Reuses existing lazy loading code for search which might otherwise be very similar between legacy search and Vue search.

Cons:

  • There is/would be an implicit dependency of having a search input with ID searchInput for both legacy search and Vue search but that might be a good thing since gadgets/extensions could rely on this id.
  • wgUseVueSearch config appears in HTML which has caching implications for anon users. For example, going to Vue search and switching back to legacy search might be difficult (maybe we would need to do this if there was a regression for example).

Option #2: Modularize the code that lazy loads search so that we can replace it with code that lazy loads the new Vue search

Core Patch: https://gerrit.wikimedia.org/r/591171
Vector Patch: https://gerrit.wikimedia.org/r/591172

Pros:

  • Caching implications in option #1 are removed (would be easier to switch back to legacy search)
  • Could deviate from the legacy lazy loading search code if needed.

Cons:

  • Adds more modules to Resources.php which increase RL startup module payload.
  • A little bit more involved to opt-in.

Option #3: Use a different ID other than 'searchInput`
This is probably not a realistic option given that it's discounted in the task description, but throwing it out there as an option for latest Vector. In latest Vector, we could just change the search input ID to not be searchInput as the lazy loading code for legacy search relies on this ID.

Pros:

  • Simple and doesn't require touching mediawiki.page.ready

Cons:

  • Lazy loading code for legacy search would still be downloaded but not used when latest Vector is on.
  • As mentioned in task description, gadgets/other things may rely on #searchInput being present.

What does the roadmap look like for removing the existing code?

As mentioned in T249051#6064322, I think if we want to be successful at meeting the requirements of DIP in a timely manner AND cutting down on the number of search widgets, we will have to do this incrementally rather than in one big swoop:

  1. Build new search for latest Vector. Latest Vector is the only client of this search at first.
  2. If above is successful, replace older search implementations with Vue search. For example and perhaps in a different order, replace legacy Vector search with Vue search, then replace Minerva search with Vue search, then replace Timeless search with Vue search (if desired), etc.

Should we plan to make the new search component apply to all skins or will we retain the old code? Where will that code live? Who will maintain it?

I think we should strive to cut down on the number of different search widgets where it makes sense. In an ideal world, we would consolidate all the usages to use the new Vue, but I think we should see where the incremental path takes us and adjust as necessary.

If anyone has any alternative recommendations or questions, feel free to post here!

[1] https://www.mediawiki.org/wiki/User:JDrewniak_(WMF)/notes/Search_widgets_at_Wikimedia

Thanks, @nray, for tackling this challenging, nebulous task! Just wanted to leave a quick comment that a separate repo works for me in the interest of moving this task forward

I think it makes sense to setup a centralized and standalone npm library repo for these components now

This sounds good to me

Per our slack conversation, @Krinkle pointed me towards an option 4 which is use 'skinScripts' - https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/Vector/+/591437/ for "What changes in core are needed to support Vector (and in future Minerva) changing the search implementation?". I've opened some tickets that would need to be tackled to unblock that path T250851 / T250853. This path also helps address some longstanding issues in Minerva with this module! Thanks for exploring all our options here. It seems like we are a little spoilt for choice!

Feels ready for sign off to me. When signing off let's make sure any tasks are created as appropriate.
Whoever's signing off should also check in with @Volker_E @Jdrewniak and @phuedx to see if they have any thoughts.

nray added a comment.Apr 27 2020, 5:40 PM

For whoever signs this off or reviews this ticket, please note Jon's comment about "skinScripts" at T249051#6076675. It is probably the most idiomatic option for how we should go about replacing the mediawiki.searchSuggest module and better than any of the POC's that I presented in T249051#6073771!

nray removed nray as the assignee of this task.Thu, Apr 30, 9:24 PM

@Jdlrobson, @nray, I've added a bullet to T249306 to capture loading only one search or another (feel free to edit!). I just wanted to say thank you so much for digging into this That was super helpful.

nray assigned this task to Volker_E.Wed, May 6, 5:10 PM
holger.knust closed this task as Resolved.EditedWed, May 6, 7:49 PM
holger.knust added a subscriber: holger.knust.
Volker_E reassigned this task from Volker_E to nray.Wed, May 6, 8:04 PM
Volker_E removed a project: Patch-For-Review.