Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: parameterized routes do not play animations or set props on vue 3.4+ #28774

Closed
3 tasks done
lvzhenbo opened this issue Jan 3, 2024 · 23 comments · Fixed by #28846
Closed
3 tasks done

bug: parameterized routes do not play animations or set props on vue 3.4+ #28774

lvzhenbo opened this issue Jan 3, 2024 · 23 comments · Fixed by #28846
Labels
package: vue @ionic/vue package type: bug a confirmed bug report

Comments

@lvzhenbo
Copy link

lvzhenbo commented Jan 3, 2024

Prerequisites

Ionic Framework Version

v7.x

Current Behavior

The inability to jump is simply the original behavior of the vue-router

Expected Behavior

Should have jumped.

Steps to Reproduce

Update vue to 3.4, update @vitejs/plugin-vue to 5.0

Code Reproduction URL

https://github.com/lvzhenbo/tsdm-app/tree/652cd1de3281772d6bdd2642d37b73567918efbc

Ionic Info

[WARN] Error loading @capacitor/ios package.json: Error: Cannot find module '@capacitor/ios/package.json'

   Require stack:
   -
   C:\Users\29422\Desktop\tsdm-app\node_modules\.pnpm\@[email protected]\node_modules\@ionic\cli\lib\project\index.js
   - C:\Users\29422\Desktop\tsdm-app\node_modules\.pnpm\@[email protected]\node_modules\@ionic\cli\lib\index.js
   - C:\Users\29422\Desktop\tsdm-app\node_modules\.pnpm\@[email protected]\node_modules\@ionic\cli\index.js
   - C:\Users\29422\Desktop\tsdm-app\node_modules\.pnpm\@[email protected]\node_modules\@ionic\cli\bin\ionic

Ionic:

Ionic CLI : 7.2.0
Ionic Framework : @ionic/vue 7.6.2

Capacitor:

Capacitor CLI : 5.6.0
@capacitor/android : 5.6.0
@capacitor/core : 5.6.0
@capacitor/ios : not installed

Utility:

cordova-res : not installed globally
native-run : not installed globally

System:

NodeJS : v20.9.0 (C:\Program Files\nodejs\node.exe)
npm : 10.1.0
OS : Windows 10

Additional Information

No response

@ionitron-bot ionitron-bot bot added the holiday triage issues that were created during holiday period label Jan 3, 2024
Copy link

ionitron-bot bot commented Jan 3, 2024

Thanks for the issue! This issue has been labeled as holiday triage. With the winter holidays quickly approaching, much of the Ionic Team will soon be taking time off. During this time, issue triaging and PR review will be delayed until the team begins to return. After this period, we will work to ensure that all new issues are properly triaged and that new PRs are reviewed. In the meantime, please read our Winter Holiday Triage Guide for information on how to ensure that your issue is triaged correctly. Thank you!

@liamdebeasi
Copy link
Contributor

Hi there,

Can you please create a reproduction of the issue that includes only the code required to reproduce the problem?

@liamdebeasi liamdebeasi added ionitron: needs reproduction a code reproduction is needed from the issue author and removed holiday triage issues that were created during holiday period labels Jan 3, 2024
Copy link

ionitron-bot bot commented Jan 3, 2024

Thanks for the issue! This issue has been labeled as needs reproduction. This label is added to issues that need a code reproduction.

Please reproduce this issue in an Ionic starter application and provide a way for us to access it (GitHub repo, StackBlitz, etc). Without a reliable code reproduction, it is unlikely we will be able to resolve the issue, leading to it being closed.

If you have already provided a code snippet and are seeing this message, it is likely that the code snippet was not enough for our team to reproduce the issue.

For a guide on how to create a good reproduction, see our Contributing Guide.

@lvzhenbo
Copy link
Author

lvzhenbo commented Jan 3, 2024

Hi there,

Can you please create a reproduction of the issue that includes only the code required to reproduce the problem?

https://github.com/lvzhenbo/ionic-vue3.4

There are two commits here, the latest commit is the incorrect case and the penultimate commit is the correct case. Then it's a matter of clicking on the selection bar to switch routes, and you'll see some one page will refresh and the other won't

@lvzhenbo
Copy link
Author

lvzhenbo commented Jan 3, 2024

I was creating the demo and realized that it may have nothing to do with the vue version update and everything to do with the @vitejs/plugin-vue version update

Hi there,
Can you please create a reproduction of the issue that includes only the code required to reproduce the problem?

lvzhenbo/ionic-vue3.4

There are two commits here, the latest commit is the incorrect case and the penultimate commit is the correct case. Then it's a matter of clicking on the selection bar to switch routes, and you'll see some one page will refresh and the other won't

@liamdebeasi
Copy link
Contributor

Updating from Vue 3.3.13 to Vue 3.4.4 causes the issue to appear. However, I'm still able to reproduce the issue if I remove Ionic Framework from the application. Have you tried reporting this to the Vue team?

@liamdebeasi liamdebeasi added needs: reply the issue needs a response from the user and removed ionitron: needs reproduction a code reproduction is needed from the issue author labels Jan 3, 2024
@lvzhenbo
Copy link
Author

lvzhenbo commented Jan 3, 2024

Updating from Vue 3.3.13 to Vue 3.4.4 causes the issue to appear. However, I'm still able to reproduce the issue if I remove Ionic Framework from the application. Have you tried reporting this to the Vue team?

For vue-router this anomaly is instead normal. Related documentation: https://router.vuejs.org/guide/essentials/dynamic-matching.html#Reacting-to-Params-Changes

So I'm guessing that ionic has a workaround for this situation, which is now broken due to a vue version upgrade

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Jan 3, 2024
@ih84ds
Copy link

ih84ds commented Jan 7, 2024

Updating from Vue 3.3.13 to Vue 3.4.4 causes the issue to appear. However, I'm still able to reproduce the issue if I remove Ionic Framework from the application. Have you tried reporting this to the Vue team?

Confirmed that downgrading my project from vue 3.4 to 3.3 resolved this issue. I was not able to find a workaround so that I could stay on 3.4.

It seems vue-router's onBeforeRouteUpdate() is firing and the to route has the correct params... but the props are not being updated with the new parameters. I'm not sure how to coerce the props into seeing the new parameters. Seems like there should be a way...

@torgnywalin
Copy link

torgnywalin commented Jan 16, 2024

We had a similar experience. When upgrading to vue 3.4.5 it started to reuse components when routing between two paths using same route with different params (user/1, user/2). Downgrading to 3.3.8 (the version we had before upgrade) solved the issue.

To me it seems like vue is following the vue-router documentation:

  • routes will reuse components when route changes
  • using watcher or onBeforeRouteUpdate to update the component is working.
  • Since component is reused we dont get ionic animations on navigation.

@maelp
Copy link

maelp commented Jan 17, 2024

Interested in knowing when this will be solved. Is this a vue issue, or is this that the ionic-vue-router should be updated to conform to the new vue-router ?

@liamdebeasi
Copy link
Contributor

liamdebeasi commented Jan 17, 2024

Is this a vue issue, or is this that the ionic-vue-router should be updated to conform to the new vue-router ?

I'm looking into this, but I don't have a good answer at the moment other than that the expected behavior should work. My initial investigation revealed that this watch callback is not firing anymore on route changes:

watch(
() => [route, matchedRouteRef.value],

Doing watch(() => [route]... does not work, but watch(route... does. I need to dig a bit more into why this is the case.


If you need a resolution sooner then I recommend downgrading to Vue 3.3.x for now.

@liamdebeasi
Copy link
Contributor

liamdebeasi commented Jan 17, 2024

It appears this new behavior is due to a breaking change in Vue: vuejs/core#9916 (comment)

The commit for this has been reverted because it was a bigger breaking change than the team expected: vuejs/core#9965 (comment)

It sounds like this was already released, so maybe there's something else causing an issue too.

@lvzhenbo
Copy link
Author

It appears this new behavior is due to a breaking change in Vue: vuejs/core#9916 (comment)

The commit for this has been reverted because it was a bigger breaking change than the team expected: vuejs/core#9965 (comment)

It sounds like this was already released, so maybe there's something else causing an issue too.

From the changelog, is it related to this pr (vuejs/core#5912) ?

@liamdebeasi liamdebeasi changed the title bug: vue3.4 the same component with reference route can not jump the problem bug: parameterized routes do not play animations or set props on vue 3.4+ Jan 17, 2024
@liamdebeasi liamdebeasi added package: vue @ionic/vue package type: bug a confirmed bug report labels Jan 17, 2024
@ionitron-bot ionitron-bot bot removed the triage label Jan 17, 2024
@liamdebeasi
Copy link
Contributor

Yeah that looks like it is related too. I'll start looking into how we can mitigate this inside of Ionic.

@liamdebeasi
Copy link
Contributor

liamdebeasi commented Jan 17, 2024

Can everyone try the following dev build and let me know if the issue is resolved in their apps with Vue 3.4+?

npm install @ionic/[email protected] @ionic/[email protected]

If you're using Vite you might need to remove node_modules/.vite before testing (this is a cache and will be re-created when you serve the app again).

@lvzhenbo
Copy link
Author

https://github.com/lvzhenbo/ionic-vue3.4
Validated, jumps fine, should be solved

@ih84ds
Copy link

ih84ds commented Jan 17, 2024

Can everyone try the following dev build and let me know if the issue is resolved in their apps with Vue 3.4+?

Confirmed. That dev build works with [email protected] for the routes I was experiencing the issue on.

@maelp
Copy link

maelp commented Jan 17, 2024

Nice! Will this be released in the next ionic update?

@mneciunas
Copy link

mneciunas commented Jan 18, 2024

Can everyone try the following dev build and let me know if the issue is resolved in their apps with Vue 3.4+?

I can confirm as well, fixed the issue for me.

github-merge-queue bot pushed a commit that referenced this issue Jan 19, 2024
Issue number: resolves #28774

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

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](vuejs/core#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](vuejs/core#9965 (comment)).
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?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- 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

<!--
  If this introduces a breaking change:
1. Describe the impact and migration path for existing applications
below.
  4. Update the BREAKING.md file with the breaking change.
5. Add "BREAKING CHANGE: [...]" to the commit description when merging.
See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#footer
for more information.
-->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

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.
@liamdebeasi
Copy link
Contributor

Thanks for the issue. This has been resolved via #28846, and a fix will be available in an upcoming release of Ionic Framework.

@maelp
Copy link

maelp commented Jan 23, 2024

Thanks! Do you know when the release will be shipped?

@liamdebeasi
Copy link
Contributor

The bug fix will follow our patch release schedule: https://ionicframework.com/docs/reference/versioning#release-schedule

Copy link

ionitron-bot bot commented Feb 22, 2024

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Feb 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: vue @ionic/vue package type: bug a confirmed bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants