Page MenuHomePhabricator

Migrate Termbox to Vue 3
Closed, ResolvedPublic

Description

This task tracks the migration of Termbox to Vue 3. The first part is making it compatible with the Vue 3 migration build (this requires dropping the vue-class-components library); afterwards, we also want to fully migrate to Vue 3, at build time as well.

Details

SubjectRepoBranchLines +/-
mediawiki/extensions/Wikibasemaster+1 -1
wikibase/termboxmaster+118 -45
operations/deployment-chartsmaster+2 -2
wikibase/termboxmaster+3 -10
wikibase/termboxmaster+1 -1
wikibase/termboxmaster+11 -44
wikibase/termboxmaster+44 -44
wikibase/termboxmaster+0 -12
wikibase/termboxmaster+7 K -2 K
wikibase/termboxmaster+160 -197
wikibase/termboxmaster+7 -1
wikibase/termboxmaster+229 -2
wikibase/termboxmaster+2 -3
wikibase/termboxmaster+55 -30
wikibase/termboxmaster+4 -5
wikibase/termboxmaster+8 -1
wikibase/termboxmaster+10 -9
wikibase/termboxmaster+15 -10
wikibase/termboxmaster+18 -10
mediawiki/extensions/Wikibasemaster+1 -1
mediawiki/extensions/Wikibasemaster+1 -1
wikibase/termboxmaster+86 -24
wikibase/termboxmaster+6 -2
wikibase/termboxmaster+12 -5
operations/deployment-chartsmaster+2 -2
wikibase/termboxmaster+92 -195
wikibase/termboxmaster+51 K -143
wikibase/termboxmaster+33 -60
wikibase/termboxmaster+379 -152
wikibase/termboxmaster+7 -36
wikibase/termboxmaster+259 -280
wikibase/termboxmaster+137 -139
wikibase/termboxmaster+33 -25
wikibase/termboxmaster+74 -89
Show related patches Customize query in gerrit

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change 742154 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[wikibase/termbox@master] Split buildApp into buildAppMw and buildAppSsr

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

Change 742167 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[operations/deployment-charts@master] Update termbox to 2021-11-26-093451-production

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

Change 742135 merged by jenkins-bot:

[wikibase/termbox@master] Simplify store initialization (asyncData in buildApp)

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

Change 742167 merged by jenkins-bot:

[operations/deployment-charts@master] Update termbox to 2021-11-26-093451-production

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

Change 742767 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[wikibase/termbox@master] Avoid using v-model

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

Change 742768 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[wikibase/termbox@master] Don\u2019t crash if Vue.config.ignoredElements is undefined

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

Change 742767 merged by jenkins-bot:

[wikibase/termbox@master] Avoid using v-model

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

Change 742768 merged by jenkins-bot:

[wikibase/termbox@master] Don\u2019t crash if Vue.config.ignoredElements is undefined

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

Change 743203 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/Wikibase@master] Update Termbox (partial Vue 3 migration)

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

Change 742154 merged by jenkins-bot:

[wikibase/termbox@master] Split buildApp into buildAppMw and buildAppSsr

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

Change 743203 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Update Termbox (partial Vue 3 migration)

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

Lucas_Werkmeister_WMDE renamed this task from Remove vue-class-components from Termbox to Migrate Termbox to Vue 3.Dec 3 2021, 12:50 PM
Lucas_Werkmeister_WMDE updated the task description. (Show Details)

Change 744852 had a related patch set uploaded (by Catrope; author: Catrope):

[mediawiki/extensions/Wikibase@master] Update Termbox (next stage of Vue 3 migration)

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

Change 744852 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Update Termbox (next stage of Vue 3 migration)

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

We've noticed problems with .native event bindings not working with compiled Vue 2 code running in the Vue 3 migration build, see https://gerrit.wikimedia.org/r/c/wvui/+/745576 . The only two uses of .native that codesearch turned up were in WVUI (which that patch fixes) and here in Termbox. I recommend testing those specific event handlers to see if they still work.

You’re right, they don’t seem to be working properly. Thanks for noticing!

Change 745771 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[wikibase/termbox@master] Don\u2019t use v-on:.native in AliasesEdit

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

Change 745771 merged by jenkins-bot:

[wikibase/termbox@master] Don\u2019t use v-on:.native in AliasesEdit

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

Note: I neglected to add this task to several other recent Termbox changes; most of the commits between 502a4ed963660183233fa6ce8de0274612c84df9 and f5649a8fab0d6e5f9191a9938fdff6c39bbe10ac are also related to this task (Gitiles log)

Change 753705 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[wikibase/termbox@master] Remove client/directives/index.ts

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

Change 753706 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[wikibase/termbox@master] Move directives and mixins to buildApp

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

Change 753710 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[wikibase/termbox@master] Move directives/ from src/client/ to src/

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

Change 753735 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[wikibase/termbox@master] Prepare JestCustomEnvironment for Jest 27

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

Change 753705 merged by jenkins-bot:

[wikibase/termbox@master] Remove client/directives/index.ts

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

Change 753736 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[wikibase/termbox@master] Replace Vue.nextTick() with wrapper.vm.$nextTick() in tests

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

Change 753770 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[wikibase/termbox@master] Widen a few SSR-related types

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

Change 753797 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[wikibase/termbox@master] Use Vue 3 at build time

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

Change 753798 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[wikibase/termbox@master] Remove no-longer-needed ts-ignore comments

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

Change 753799 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[wikibase/termbox@master] Change propsData to props in tests

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

Change 753800 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[wikibase/termbox@master] Merge buildApp functions again

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

Change 753801 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[wikibase/termbox@master] Rename Jest config globals.ts-jest.tsConfig to tsconfig

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

Change 753802 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[wikibase/termbox@master] Simplify server tsconfig

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

Change 753706 merged by jenkins-bot:

[wikibase/termbox@master] Move directives and mixins to buildApp

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

Change 753710 merged by jenkins-bot:

[wikibase/termbox@master] Move directives/ from src/client/ to src/

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

Change 753735 merged by jenkins-bot:

[wikibase/termbox@master] Prepare tests for Jest 27

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

Change 753736 merged by jenkins-bot:

[wikibase/termbox@master] Replace Vue.nextTick() with wrapper.vm.$nextTick() in tests

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

Change 753770 merged by jenkins-bot:

[wikibase/termbox@master] Widen a few SSR-related types

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

Change 753976 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[wikibase/termbox@master] Avoid v-model in Checkbox story

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

Change 753999 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[wikibase/termbox@master] Load all of @storybook/addon-essentials, not just docs

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

Change 753976 merged by jenkins-bot:

[wikibase/termbox@master] Avoid v-model in Checkbox story

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

Change 753999 merged by jenkins-bot:

[wikibase/termbox@master] Load all of @storybook/addon-essentials, not just docs

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

Change 754550 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[wikibase/termbox@master] WIP: Use getSSRProps instead of directiveTransforms

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

Change 754550 abandoned by Lucas Werkmeister (WMDE):

[wikibase/termbox@master] WIP: Use getSSRProps instead of directiveTransforms

Reason:

I just wanted to have this up on Gerrit, it’s not actually meant for review.

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

Change 753797 merged by jenkins-bot:

[wikibase/termbox@master] Use Vue 3 at build time

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

Change 753798 merged by jenkins-bot:

[wikibase/termbox@master] Remove no-longer-needed ts-ignore comments

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

Change 753799 merged by jenkins-bot:

[wikibase/termbox@master] Change propsData to props in tests

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

Change 753800 merged by jenkins-bot:

[wikibase/termbox@master] Merge buildApp functions again

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

Change 753801 merged by jenkins-bot:

[wikibase/termbox@master] Rename Jest config globals.ts-jest.tsConfig to tsconfig

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

Change 753802 merged by jenkins-bot:

[wikibase/termbox@master] Simplify server tsconfig

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

Next up: update the submodule in Wikibase.git and the image in deployment-charts.

Change 757412 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/Wikibase@master] Update Termbox (full Vue 3 migration)

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

The above change doesn’t work properly yet:

Hydration completed but contains mismatches.

(You can get more detailed messages with ?debug.) I think this is because of the changed mounting behavior in Vue 3. The SSR contains HTML like <section class="wikibase-entitytermsview">, and Wikibase adds that to its server-rendered HTML for something like this:

<div class="wikibase-entityview-main">
  <section class="wikibase-entitytermsview"></section>
  <div id="toc"></div>
  <h2 class="wb-section-heading section-heading wikibase-statements" dir="auto"><span class="mw-headline" id="claims">Statements</span></h2>
  <div class="wikibase-statementgrouplistview"></div>
</div>

Then, when hydrating the termbox, we call app.mount( '.wikibase-entitytermsview' ), which under Vue 2 was expected to replace the element matching that selector (the <section>). In Vue 3, this tries to mount the app inside / below that element instead, which produces the hydration mismatches, but also the following DOM when the hydration is finished:

<section class="wikibase-entitytermsview" dir="ltr" lang="en">
  <div class="wb-ui-termbox">
    <section class="wikibase-entitytermsview" dir="ltr" lang="en">
      <div class="wb-ui-termbox"></div>
    </section>
  </div>
</section>

We can’t just change the .mount() call to use the selector .wikibase-entityview-main instead, because that would wipe the other content in there (the statements). I think we need to introduce a new wrapper div. And I think we need to do that in Wikibase.git (not in the SSR), so that we can deploy these changes at the same time:

  • Wikibase PHP code emits HTML with an extra div around the <section>
  • Termbox JS code (Wikibase submodule) mounts under that extra div

And I think we’ll want to make sure that this div is in the part of the HTML that doesn’t end up in the parser cache.

And I think we’ll want to make sure that this div is in the part of the HTML that doesn’t end up in the parser cache.

I discussed this a bit with @Tarrow and it might be trickier than expected. Apparently, the termbox HTML sometimes ends up in the parser cache (there is some PlaceholderExpander mechanism, but it’s not expected to be used all the time?), and there is also a termboxVersion option by which the parser cache may be split. @Jakob_WMDE would probably know more in this area. However, I don’t think we’ll have time to figure this out fully before the lexicographical data hike starts, so I think this might have to wait until March or so.

Fortunately, this doesn’t need to block the Termbox Vue 3 rollout at all. For now, we can add a wrapper element in our own JS, just before calling .mount(); the only disadvantage of this is that the server-rendered HTML lacks this extra element, but that difference should be acceptable for a while. We can write the JS code in such a way that it detects the wrapper if it’s already there, and then add the wrapper on the PHP side later, when we figure out how best to do it. (If the JS can handle HTML with or without wrapper, that should also make the PHP side easier – we might not have to worry about the parser cache as much in that case.)

So: for now, make the JS add the wrapper element inside which we mount the app; later, also make the PHP add this wrapper element to the HTML it generates. (I don’t think we want to add this HTML to the SSR-generated HTML, that should still start with <section>.)

Change 757504 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[wikibase/termbox@master] Update buildAndAttemptHydration for Vue 3

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

Change 757659 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[operations/deployment-charts@master] Update termbox to 2022-01-25-175409-production

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

Change 757659 merged by jenkins-bot:

[operations/deployment-charts@master] Update termbox to 2022-01-25-175409-production

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

Change 757504 merged by jenkins-bot:

[wikibase/termbox@master] Update buildAndAttemptHydration for Vue 3

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

Change 757412 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Update Termbox (full Vue 3 migration)

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

The final stage should roll out with this week’s train (T300197, I don’t think it’s worth mentioning as a risky change there) – if everything works, then I think we can close this task (despite the open subtask T300176).