Use one vendored copy of vue-virtual-scroller

Miguel Rinconrequested to merge
vue3-single-vue-virtual-scroller-copy into master

What this does

We vendor two copies of vue-virtual-scroller. CONTEXT_ALIASES redirects Vue 3 pages to the second one. That copy crashes under @vue/compat.

vendor/assets/javascripts/vue-virtual-scroller-vue3/src/components/DynamicScrollerItem.vue:245 calls this.$slots.default(). That is a function only under a real Vue 3 runtime. Under @vue/compat it is an array, so the webpack build threw as soon as a page opted into Vue 3 with Option 1:

TypeError: this.$slots.default is not a function

This MR drops the vendor/vue-virtual-scroller redirect and deletes that copy. Every page now uses vendor/assets/javascripts/vue-virtual-scroller, which the rest of the application already uses on Vue 2 pages.

Why remove rather than patch

  • Every prop the call sites pass exists on the remaining copy: items, keyField, itemSize, minItemSize, buffer, pageMode, useTransform, and on DynamicScrollerItem item, active, watchData, sizeDependencies, emitResize, tag.
  • It revives a dead binding. app/assets/javascripts/diffs/components/app.vue:893 passes :use-transform="false". Only the remaining copy implements that prop; the deleted one renamed it to disableTransform, so the binding did nothing whenever the diffs page ran under Vue 3.
  • The deleted copy was not the release it claimed. Its package.json said 2.0.0-beta.8, but the published 2.0.0-beta.8 contains neither disableTransform nor useTransform. It was a snapshot of the 2.x master branch taken in e90f862b, carrying a local change to a prop default.
  • Nothing else referenced it once the redirect was gone.

Why not upgrade instead

Moving to a current upstream release is separate, larger work, and the vendoring model is most of the reason.

We cannot vendor the current source easily, because it is TypeScript. Upstream converted the library after our snapshot was taken. At 2.0.0-beta.9, 21 of the 29 source files are .ts and the five SFCs are <script setup lang="ts">. Our infection matcher is:

const INFECTABLE_RE = /\.(js|mjs|vue)$/;

.ts does not match, so the infection would never propagate into the composables, which is where all the logic lives after the v2 rewrite. That is the same class of failure that produced this bug. Widening the regex is possible, but it is a change to shared Vue 3 migration infrastructure affecting every page, not a dependency bump.

Taking the built output instead does not work either. From 2.0.0-beta.9 onwards npm ships dist only, with no src. That is a single pre-compiled ESM bundle importing vue directly, which bypasses the ?vue3 model entirely.

So an upgrade means either vendoring TypeScript from a git tag and teaching the infection tooling about it, or stopping vendoring altogether. Both are worth doing deliberately rather than as part of a crash fix.

On top of that, every candidate version carries the same two migrations: the IdState mixin is gone from 2.0.x onwards, and disableTransform reverts to the upstream default against our patched one. #619357 (closed) has the full comparison.

Verification

  • In GDK with vue3_migrate_artifacts on, the artifacts page serves pages.projects.artifacts.vue3.js, mounts CiArtifactsRoot, and expanding a job row renders the scroller (vue-recycle-scroller ready direction-vertical) with no $slots error. Before this change the same interaction threw on every expand.

MR stack

Goal MR
Harden tests / guarantee fixes work Add feature spec coverage for the artifacts page (!251287 - merged)
Clean up faulty dependency Use one vendored copy of vue-virtual-scroller (!251351 - merged)
Fix Vite Vue 3 infection Align Vite and webpack Vue 3 alias resolution (!251350 - merged)
Migration of artifacts entrypoints Add Vue 3 rollout flag for the artifacts pages (!250423 - merged)

References

Edited by Miguel Rincon

Merge request reports