Guard GlDatepicker teardown when it is destroyed before mount

Miguel Rinconrequested to merge
datepicker-guard-destroy-before-mount into main

What does this MR do?

Why

GlDatepicker wraps the Pikaday calendar library. It creates the Pikaday instance in mounted and stores it as this.calendar. The beforeDestroy hook and the five prop watchers (value, minDate, maxDate, startRange, endRange) call methods on calendar without checking that it exists.

Under Vue 3 (GitLab runs it through @vue/compat), mounted is a post-flush callback. A component created and torn down in the same tick runs beforeDestroy before mounted. A prop change queued in that same flush runs a watcher before mounted. In both cases calendar is undefined and the call throws TypeError: Cannot read properties of undefined (reading 'destroy'). Vue's compat runtime then follows with TypeError: Cannot read properties of null (reading 'emitsOptions'). Under Vue 2, mounted runs synchronously, so this never happens.

This affects any Vue 3 page that renders a datepicker inside content that re-renders on a route or filter change. Concrete case: the GitLab merge request list, filtered search "deployed before/after" date tokens.

What

Guard each access to calendar. Use this.calendar?.destroy() in beforeDestroy. The five watchers skip the Pikaday call when calendar is not set. mounted reads the current prop values when it creates the instance, so nothing is lost.

How

  • Optional chaining in the beforeDestroy hook.
  • A calendar check in the value, minDate, maxDate, startRange and endRange watchers.
  • Changeset .changeset/datepicker-destroy-before-mount.md for a @gitlab/ui patch release.

Alternatives ruled out:

  • Fix only the GitLab caller with a v-if or key on the date token: fixes one page, every other datepicker under Vue 3 keeps the bug.
  • Create Pikaday in created instead of mounted: not possible, Pikaday needs the rendered input and this.$el.
  • Also guard show() and onKeydown: not done. show() is only reachable by an explicit call on a ref, and a parent's mounted runs after its children's, so the calendar exists by then. A guard there would hide a caller bug. onKeydown only runs on user input in a rendered field.

Gotcha: the watcher guards came from a review comment on this MR and are a second commit on purpose, so the diff the first review saw is unchanged.

Background: how this surfaced
  • GitLab MR gitlab-org/gitlab!255285 (merged) makes bare Vue Router routes such as routes: [{ path: '/' }] matchable under Vue Router 4, as under Vue Router 3.
  • Its Vue 3 system spec job failed in spec/features/merge_requests/user_filters_by_deployments_spec.rb on a browser console check with the destroy TypeError. It reproduced on retry and locally with the GDK dev server on Vue 3, where module URLs pointed at datepicker.vue line 398, the beforeDestroy hook.
  • On the merge base under Vue 3, the same spec failed differently, with Uncaught Error: No match for {"query": ...} from the router. That is the bug the GitLab MR fixes.
  • The merge request list calls this.$router.push({ query }) on each filter change and has the bare route. Before the GitLab MR the push threw and nothing re-rendered. After it, the route-driven re-render creates and tears down the date token's GlDatepicker in one tick.
  • Conclusion: the router fix is correct and exposes a latent Vue 3 bug in GlDatepicker.

How to verify

From packages/gitlab-ui:

yarn test:unit src/components/base/datepicker/datepicker.spec.js
yarn test:unit-vue3 src/components/base/datepicker/datepicker.spec.js

Both runs pass 62 tests. Revert check: revert the guards in datepicker.vue and rerun the Vue 3 command. The destroy case and the value and minDate watcher cases fail.

Integration check: build this branch into GitLab, switch the dev server to Vue 3, and run spec/features/merge_requests/user_filters_by_deployments_spec.rb:68:80 on top of GitLab MR gitlab-org/gitlab!255285 (merged). Without this fix both examples fail on the destroy console error.

Screenshots or screen recordings

No visual change.

Integrations

  • GitLab: no integration change needed. A Renovate dependency update will pick up the patch.

References

🤖 Generated with Claude Code

Edited by Miguel Rincon

Merge request reports