From 5bc439961fda2c595666e67688fdd1b8ff4dd0bf Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Fri, 19 Jan 2024 16:29:56 -0500 Subject: [PATCH] fix(vue): tabs and parameterized routes work with latest vue (#28846) Issue number: resolves #28774 --------- ## What is the current behavior? There are two issues causing Ionic Vue apps to not behave as intended with certain versions of Vue: 1. In Vue 3.3 a [breaking change shipped](https://github.com/vuejs/core/issues/9916) that changes the default behavior of the `watch` inside of IonRouterOutlet to be a shallow watcher instead of a deep watcher. This caused the router outlet to not consistent re-render. While the change was later reverted by the Vue team, they expressed that the change [may re-land in a future minor release](https://github.com/vuejs/core/issues/9965#issuecomment-1875067499). As a result, we will need to account for this inside of Ionic. 2. In Vue 3.2 a [custom elements improvement shipped](https://github.com/vuejs/core/blob/main/changelogs/CHANGELOG-3.2.md#3238-2022-08-30) that changed how custom elements are referred to in VNodes. ## What is the new behavior? - The affected `watch` call now is now explicitly a deep watcher. This change is backwards compatible as well as forward compatible with upcoming Vue changes. - Updated IonTabs to account for the new VNode behavior for custom elements. Ionic still supports version of Vue that do not have this improvement, so we need to account for both behaviors for now. I also added a tech debt ticket to remove the old checks when we drop support for older versions of Vue. - Updated E2E test dependencies. During this update some of our tests needed to be updated to account for newer versions of Vue/Vitest. Overall I was able to simplify a lot of our tests as a result. I plan to add renovatebot to these E2E test apps, but I will handle that in a separate PR. ## Does this introduce a breaking change? - [ ] Yes - [x] No ## Other information Dev build: `7.6.6-dev.11705526292.1bc0acb5` Note: Both of the issues cause tests to fail when using the latest dependencies in the Vue E2E test app. However, I need to use the latest dependencies so I can demonstrate that my changes do fix the reported issues. As a result, I have both fixes in the same PR. --- .../vue/src/components/IonRouterOutlet.ts | 10 +++++- packages/vue/src/components/IonTabs.ts | 35 ++++++++++++++----- packages/vue/test/apps/vue3/package.json | 6 ++-- .../vue/test/base/tests/unit/hooks.spec.ts | 17 ++++----- .../test/base/tests/unit/lifecycle.spec.ts | 13 +++---- .../vue/test/base/tests/unit/page.spec.ts | 11 ++---- .../base/tests/unit/router-outlet.spec.ts | 12 ++----- .../vue/test/base/tests/unit/routing.spec.ts | 33 ++++++++--------- .../vue/test/base/tests/unit/tab-bar.spec.ts | 22 +++++------- .../vue/test/base/tests/unit/tabs.spec.ts | 15 +++----- 10 files changed, 81 insertions(+), 93 deletions(-) diff --git a/packages/vue/src/components/IonRouterOutlet.ts b/packages/vue/src/components/IonRouterOutlet.ts index 4b4c634e946..1c32de13c01 100644 --- a/packages/vue/src/components/IonRouterOutlet.ts +++ b/packages/vue/src/components/IonRouterOutlet.ts @@ -115,7 +115,15 @@ export const IonRouterOutlet = /*@__PURE__*/ defineComponent({ previousMatchedRouteRef = currentMatchedRouteRef; previousMatchedPath = currentRoute.path; - } + }, + /** + * Future versions of Vue may default watching nested + * reactive objects to "deep: false". + * We explicitly set this watcher to "deep: true" to + * account for that. + * https://github.com/vuejs/core/issues/9965#issuecomment-1875067499 + */ + { deep: true } ); const canStart = () => { diff --git a/packages/vue/src/components/IonTabs.ts b/packages/vue/src/components/IonTabs.ts index a2333695a30..432c2479b59 100644 --- a/packages/vue/src/components/IonTabs.ts +++ b/packages/vue/src/components/IonTabs.ts @@ -6,6 +6,28 @@ const DID_CHANGE = "ionTabsDidChange"; // TODO(FW-2969): types +/** + * Vue 3.2.38 fixed an issue where Web Component + * names are respected using kebab case instead of pascal case. + * As a result, we need to account for both here since we support + * versions of Vue < 3.2.38. + */ +// TODO FW-5904 +const isRouterOutlet = (node: VNode) => { + return ( + node.type && + ((node.type as any).name === "IonRouterOutlet" || + node.type === "ion-router-outlet") + ); +}; + +const isTabBar = (node: VNode) => { + return ( + node.type && + ((node.type as any).name === "IonTabBar" || node.type === "ion-tab-bar") + ); +}; + export const IonTabs = /*@__PURE__*/ defineComponent({ name: "IonTabs", emits: [WILL_CHANGE, DID_CHANGE], @@ -19,9 +41,8 @@ export const IonTabs = /*@__PURE__*/ defineComponent({ * inside of ion-tabs. */ if (slottedContent && slottedContent.length > 0) { - routerOutlet = slottedContent.find( - (child: VNode) => - child.type && (child.type as any).name === "IonRouterOutlet" + routerOutlet = slottedContent.find((child: VNode) => + isRouterOutlet(child) ); } @@ -57,13 +78,11 @@ export const IonTabs = /*@__PURE__*/ defineComponent({ * since that needs to be inside of `.tabs-inner`. */ const filteredContent = slottedContent.filter( - (child: VNode) => - !child.type || - (child.type && (child.type as any).name !== "IonRouterOutlet") + (child: VNode) => !child.type || !isRouterOutlet(child) ); - const slottedTabBar = filteredContent.find( - (child: VNode) => child.type && (child.type as any).name === "IonTabBar" + const slottedTabBar = filteredContent.find((child: VNode) => + isTabBar(child) ); const hasTopSlotTabBar = slottedTabBar && slottedTabBar.props?.slot === "top"; diff --git a/packages/vue/test/apps/vue3/package.json b/packages/vue/test/apps/vue3/package.json index 669d1600d8b..09e9b8fcdf6 100644 --- a/packages/vue/test/apps/vue3/package.json +++ b/packages/vue/test/apps/vue3/package.json @@ -19,8 +19,8 @@ "@ionic/vue": "^6.0.12", "@ionic/vue-router": "^6.0.12", "ionicons": "^6.0.4", - "vue": "^3.2.31", - "vue-router": "^4.0.14" + "vue": "^3.4.14", + "vue-router": "^4.2.5" }, "devDependencies": { "@typescript-eslint/eslint-plugin": "^5.4.0", @@ -35,7 +35,7 @@ "jsdom": "^20.0.0", "typescript": "~4.5.5", "vite": "^3.1.4", - "vitest": "^0.23.4", + "vitest": "^1.2.1", "wait-on": "^5.3.0" }, "engines": { diff --git a/packages/vue/test/base/tests/unit/hooks.spec.ts b/packages/vue/test/base/tests/unit/hooks.spec.ts index b0ef23bed7a..d6657faa6bc 100644 --- a/packages/vue/test/base/tests/unit/hooks.spec.ts +++ b/packages/vue/test/base/tests/unit/hooks.spec.ts @@ -1,14 +1,9 @@ import { mount } from '@vue/test-utils'; import { describe, expect, it } from 'vitest'; import { createRouter, createWebHistory } from '@ionic/vue-router'; -import { IonicVue, IonApp, IonRouterOutlet, IonPage, useIonRouter, createAnimation } from '@ionic/vue'; +import { IonicVue, IonRouterOutlet, IonPage, useIonRouter } from '@ionic/vue'; import { mockAnimation, waitForRouter } from './utils'; -const App = { - components: { IonApp, IonRouterOutlet }, - template: '', -} - const BasePage = { template: '', components: { IonPage }, @@ -40,7 +35,7 @@ describe('useIonRouter', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -85,7 +80,7 @@ describe('useIonRouter', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -137,7 +132,7 @@ describe('useIonRouter', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -181,7 +176,7 @@ describe('useIonRouter', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -229,7 +224,7 @@ describe('useIonRouter', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } diff --git a/packages/vue/test/base/tests/unit/lifecycle.spec.ts b/packages/vue/test/base/tests/unit/lifecycle.spec.ts index f82106ccbe8..3c1996d5e74 100644 --- a/packages/vue/test/base/tests/unit/lifecycle.spec.ts +++ b/packages/vue/test/base/tests/unit/lifecycle.spec.ts @@ -1,15 +1,10 @@ import { mount } from '@vue/test-utils'; import { describe, expect, it, vi } from 'vitest'; import { createRouter, createWebHistory } from '@ionic/vue-router'; -import { IonicVue, IonApp, IonRouterOutlet, IonTabs, IonPage } from '@ionic/vue'; +import { IonicVue, IonRouterOutlet, IonTabs, IonPage } from '@ionic/vue'; import { defineComponent } from 'vue'; import { waitForRouter } from './utils'; -const App = { - components: { IonApp, IonRouterOutlet }, - template: '', -} - const BasePage = { template: '', components: { IonPage }, @@ -54,7 +49,7 @@ describe('Lifecycle Events', () => { // Initial render router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -108,7 +103,7 @@ describe('Lifecycle Events', () => { template: ` - + `, @@ -148,7 +143,7 @@ describe('Lifecycle Events', () => { // Initial render router.push('/tab1'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } diff --git a/packages/vue/test/base/tests/unit/page.spec.ts b/packages/vue/test/base/tests/unit/page.spec.ts index cd8917c30bd..4fb0e985f25 100644 --- a/packages/vue/test/base/tests/unit/page.spec.ts +++ b/packages/vue/test/base/tests/unit/page.spec.ts @@ -1,12 +1,7 @@ import { mount } from '@vue/test-utils'; import { describe, expect, it } from 'vitest'; import { createRouter, createWebHistory } from '@ionic/vue-router'; -import { IonicVue, IonApp, IonRouterOutlet, IonPage } from '@ionic/vue'; - -const App = { - components: { IonApp, IonRouterOutlet }, - template: '', -} +import { IonicVue, IonRouterOutlet, IonPage } from '@ionic/vue'; describe('IonPage', () => { it('should add ion-page class', async () => { @@ -25,7 +20,7 @@ describe('IonPage', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -50,7 +45,7 @@ describe('IonPage', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } diff --git a/packages/vue/test/base/tests/unit/router-outlet.spec.ts b/packages/vue/test/base/tests/unit/router-outlet.spec.ts index 89ee04bc559..f25490c2a21 100644 --- a/packages/vue/test/base/tests/unit/router-outlet.spec.ts +++ b/packages/vue/test/base/tests/unit/router-outlet.spec.ts @@ -3,22 +3,14 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { createRouter, createWebHistory } from '@ionic/vue-router'; import { IonicVue, - IonApp, IonRouterOutlet, IonPage, useIonRouter, - createAnimation } from '@ionic/vue'; -import { onBeforeRouteLeave } from 'vue-router'; import { mockAnimation, waitForRouter } from './utils'; enableAutoUnmount(afterEach); -const App = { - components: { IonApp, IonRouterOutlet }, - template: '', -} - const BasePage = { template: '', components: { IonPage }, @@ -60,7 +52,7 @@ describe('Routing', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -122,7 +114,7 @@ describe('Routing', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } diff --git a/packages/vue/test/base/tests/unit/routing.spec.ts b/packages/vue/test/base/tests/unit/routing.spec.ts index a4a8f55521a..3d1f50a428f 100644 --- a/packages/vue/test/base/tests/unit/routing.spec.ts +++ b/packages/vue/test/base/tests/unit/routing.spec.ts @@ -17,11 +17,6 @@ import { waitForRouter } from './utils'; enableAutoUnmount(afterEach); -const App = { - components: { IonApp, IonRouterOutlet }, - template: '', -} - const BasePage = { template: '', components: { IonPage }, @@ -46,7 +41,7 @@ describe('Routing', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -74,7 +69,7 @@ describe('Routing', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -107,7 +102,7 @@ describe('Routing', () => { router.push('/myPath/123'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -147,7 +142,7 @@ describe('Routing', () => { router.push('/myPath'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -182,7 +177,7 @@ describe('Routing', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -203,7 +198,7 @@ describe('Routing', () => { template: ` - + Tab 1 @@ -247,9 +242,9 @@ describe('Routing', () => { ] }); - router.push('/'); + router.push('/tabs/tab1'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -302,7 +297,7 @@ describe('Routing', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -355,7 +350,7 @@ describe('Routing', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -402,7 +397,7 @@ describe('Routing', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -450,7 +445,7 @@ describe('Routing', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -618,7 +613,7 @@ describe('Routing', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -672,7 +667,7 @@ describe('Routing', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } diff --git a/packages/vue/test/base/tests/unit/tab-bar.spec.ts b/packages/vue/test/base/tests/unit/tab-bar.spec.ts index d1c1037a924..00ded7020b4 100644 --- a/packages/vue/test/base/tests/unit/tab-bar.spec.ts +++ b/packages/vue/test/base/tests/unit/tab-bar.spec.ts @@ -1,12 +1,7 @@ import { mount } from '@vue/test-utils'; import { describe, expect, it } from 'vitest'; import { createRouter, createWebHistory } from '@ionic/vue-router'; -import { IonicVue, IonApp, IonRouterOutlet, IonPage, IonTabs, IonTabBar } from '@ionic/vue'; - -const App = { - components: { IonApp, IonRouterOutlet }, - template: '', -} +import { IonicVue, IonRouterOutlet, IonPage, IonTabs, IonTabBar } from '@ionic/vue'; describe('ion-tab-bar', () => { it('should render in the top slot', async () => { @@ -31,7 +26,7 @@ describe('ion-tab-bar', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(Tabs, { global: { plugins: [router, IonicVue] } @@ -67,7 +62,7 @@ describe('ion-tab-bar', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(Tabs, { global: { plugins: [router, IonicVue] } @@ -102,7 +97,7 @@ describe('ion-tab-bar', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(Tabs, { global: { plugins: [router, IonicVue] } @@ -141,16 +136,15 @@ describe('ion-tab-bar', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(Tabs, { global: { plugins: [router, IonicVue] } }); - const innerHTML = wrapper.find('ion-tabs').html(); - - const tabs = wrapper.findComponent(IonTabBar); - const children = tabs.vm.$el.childNodes; + const tabs = wrapper.findComponent(IonTabs); + const tabbar = tabs.vm.$el.children[1]; + const children = tabbar.childNodes; // 8 is a comment node: https://developer.mozilla.org/en-US/docs/Web/API/Node/nodeType expect(children[0].nodeType).toEqual(8); diff --git a/packages/vue/test/base/tests/unit/tabs.spec.ts b/packages/vue/test/base/tests/unit/tabs.spec.ts index 528386b8963..73a82bd8748 100644 --- a/packages/vue/test/base/tests/unit/tabs.spec.ts +++ b/packages/vue/test/base/tests/unit/tabs.spec.ts @@ -1,14 +1,9 @@ import { mount } from '@vue/test-utils'; import { describe, expect, it } from 'vitest'; import { createRouter, createWebHistory } from '@ionic/vue-router'; -import { IonicVue, IonApp, IonRouterOutlet, IonPage, IonTabs, IonTabBar, IonTabButton, IonLabel } from '@ionic/vue'; +import { IonicVue, IonRouterOutlet, IonPage, IonTabs, IonTabBar, IonTabButton, IonLabel } from '@ionic/vue'; import { waitForRouter } from './utils'; -const App = { - components: { IonApp, IonRouterOutlet }, - template: '', -} - const Tabs = { components: { IonPage, IonTabs, IonTabBar, IonTabButton, IonLabel, IonRouterOutlet }, template: ` @@ -64,7 +59,7 @@ describe('ion-tabs', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -112,7 +107,7 @@ describe('ion-tabs', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -166,7 +161,7 @@ describe('ion-tabs', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] } @@ -222,7 +217,7 @@ describe('ion-tabs', () => { router.push('/'); await router.isReady(); - const wrapper = mount(App, { + const wrapper = mount(IonRouterOutlet, { global: { plugins: [router, IonicVue] }