Stop sharing module state across the Vue 2 and Vue 3 lanes

Miguel Rinconrequested to merge
625296-fix-cross-lane-duplication into master

What does this MR do and why?

Five modules hold module-scope state in both lanes at once. Each copy re-runs the module body, so the state becomes two objects. All five already ship to users. This MR fixes them.

Module State Pages affected Fix
graphql_shared/issuable_client.js Apollo cache 14 page states split the client from the provider
pages/projects/show/index.js (4 modules) Pinia instance, file tree store, main container store, repository Apollo cache project overview one flag for the overview, tree and blob pages
behaviors/preview_markdown.js document listeners 31 page states INFECTION_BLOCKLIST
invite_members/init_invite_members_modal.js mount memo 8 pages INFECTION_BLOCKLIST

Case 1. work items Apollo client

graphql_shared/issuable_client.js built both the Apollo client and the VueApollo provider. super_sidebar loads on every page. It reaches that module through the create-work-item modal in the topbar, in super_topbar.vue then create_menu.vue then create_work_item_modal.vue then work_items/graphql/cache_utils.js. A migrated page had two caches, on 14 page states.

The client moves to graphql_shared/issuable_default_client.js. That file goes on INFECTION_BLOCKLIST, so every lane shares one cache. The provider stays in issuable_client.js. A provider cannot be shared: vue-apollo resolves to a different package per lane, and a provider built for Vue 2 does not work in a Vue 3 app.

issuable_client.js re-exports config, resolvers and defaultClient, so none of the 28 importers changes. The moved file is byte-identical to the old one, without the provider and its vue-apollo import.

Case 2. Project overview page

pages/projects/show/index.js loads the repository tree and the blob viewer through dynamic imports, each guarded by a check for its element. Both specifiers carried ?vue3, and that query does not read the flag. Whenever either one loaded it ran Vue 3, while the page entry followed vue3_migrate_project_overview and usually ran Vue 2. That put two lanes on one page and split four modules: pinia/instance.js, repository/stores/file_tree_browser_visibility.js, pinia/global_stores/main_container.js and repository/graphql.js.

The user-visible effect: header_area.vue writes the file tree visibility store, and file_tree_browser.vue reads it. They read different copies, so the toggle in the header did nothing.

Those query strings were there for a reason. pages/projects/tree/show and pages/projects/blob/show had status: migrated, so that code ran Vue 3 only, and the overview page had to match it.

The fix moves the pages instead of the imports. Both pages join vue3_migrate_project_overview with status: rollout, and both ?vue3 query strings are gone. Every module on the overview page now follows one flag, in both flag states. pages/projects/show/index.js ends up with no query strings at all.

The flag keeps default_enabled: false. The repository tree and the blob viewer therefore run Vue 2 until somebody enables the flag. That is a deliberate step back for those two pages. It also gives the whole repository browsing surface one switch, which it did not have before, because a migrated page has no flag to turn off.

Case 3. The two blocklist entries

behaviors/preview_markdown.js registers document listeners at module scope, and main loads it on every page. Two copies handled one click twice. The second handler read the button value that the first had already changed, so the markdown preview opened and closed again.

invite_members/init_invite_members_modal.js memoises the one mounted modal inside a closure. Two copies each saw an empty memo, so both mounted an app on the same element.

Neither is a candidate for a feature flag. main has no flag, so grouping pages under one flag cannot align these lanes.

Case 4. work_item_attribute_popovers.js client import

work_item_attribute_popovers.js imported the work item client only to build its own provider. It needs none of that client's resolvers: the popover runs one server query, and no document in the popover tree carries an @client directive. Its sibling path, the default export of issuable/popover/index.js, already mounts the same components with a plain createDefaultClient().

It now builds a plain client too. That drops 13 modules from the main bundle.

super_sidebar_bundle.js keeps the shared provider. An earlier revision of this MR gave it a plain client as well. That was wrong: super_topbar.vue reaches create_work_item.vue, which runs update_new_work_item.mutation.graphql, an @client mutation that only the shared resolvers answer. A plain client would have failed it in silence. Splitting the client out of issuable_client.js already removes the duplication, so the sidebar needs no change at all.

Dependency cruiser baseline

graphql_shared/issuable_client.js was on the no-shared-layer-imports-from-features exemption list. The rename moves the same imports to issuable_default_client.js, so the exemption follows the path. The list is the same length. No new exemption is added.

How to verify

yarn jest spec/frontend/super_sidebar ee/spec/frontend/super_sidebar spec/frontend/repository
yarn jest spec/frontend/invite_members spec/frontend/behaviors/preview_markdown_spec.js
yarn jest spec/frontend/config/vue3_migration_file_validation_spec.js
yarn vite-prod

1467 tests across 22 suites ran for every spec that imports the client. 1722 tests across 85 suites ran for the sidebar, invite members, preview markdown and popover specs. 1282 tests across 91 suites ran for the repository specs and the migration config validation. 202 tests ran for the Pinia, What's new and shared GraphQL specs. A production Vite build passed. ESLint and dependency cruiser pass on every changed file.

This MR is the parent of the merge request that adds the build check. That check is green against this branch: 353 EE page states and 240 CE page states, none found.

References

Edited by Miguel Rincon

Merge request reports