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: vue re-mounts intermediary views when not needed #23914

Closed
4 of 6 tasks
kabzko opened this issue Sep 11, 2021 · 22 comments · Fixed by #24056
Closed
4 of 6 tasks

bug: vue re-mounts intermediary views when not needed #23914

kabzko opened this issue Sep 11, 2021 · 22 comments · Fixed by #24056
Labels
package: vue @ionic/vue package type: bug a confirmed bug report
Milestone

Comments

@kabzko
Copy link

kabzko commented Sep 11, 2021

Prequisites

Ionic Framework Version

  • v4.x
  • v5.x
  • v6.x

Current Behavior

I have IonMenu with a list of routes.

When I push a page(Orders) for the first time all is well.
First
and again when I push to a new page for the second time it renders the new page(Products) and the old page(Orders).
Second
It affects my methods that I created for both pages. It calls all the created methods from both page.
Capture

Here's some video that I recorded
https://user-images.githubusercontent.com/41864097/132933264-769c352c-49ba-4174-9d0c-44719469d752.mp4

Expected Behavior

Old page must not be rendered again when you push to a new page

Steps to Reproduce

Just navigate from menu and check the console which I console log only text for testing.

Code Reproduction URL

https://github.com/kabzko/testroute

Ionic Info

Ionic:

Ionic CLI : 6.17.1

Utility:

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

System:

NodeJS : v14.17.0
npm : 6.14.13
OS : Windows 10

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage label Sep 11, 2021
@whitersun
Copy link

whitersun commented Sep 12, 2021

For more detail issue, please watch it.
https://drive.google.com/file/d/1gNdeJHYdGitj_ApHNeUKQFXiwG8hJTRe/view?usp=sharing

Description:
When in home page router to Products, the Products page will render two components included the Order view page.
But if we reload in child component like the Product page, then return home page. And router again to order then return the home router to products will no render also to components anymore.

image

@UrosPavlovic
Copy link

Same as for me. Currently in my new app, i have only 3 pages, home, login and register. Switching from Login to Register, vice versa, or from Home to either of them renders 2 views at same time. I can also post a video if needed.

Relevant deps:
"@ionic/vue": "^5.4.0",
"@ionic/vue-router": "^5.4.0",
"vue": "^3.2.1",
"vue-router": "^4.0.0-0",

@liamdebeasi liamdebeasi changed the title bug: IonRouterOutlet rerender old route when pushing new route bug: vue re-mounts intermediary views when not needed Sep 21, 2021
@liamdebeasi liamdebeasi added package: vue @ionic/vue package type: bug a confirmed bug report labels Sep 21, 2021
@liamdebeasi
Copy link
Contributor

Thanks for the issue. Looks like this line is the culprit: https://github.com/ionic-team/ionic-framework/blob/main/packages/vue/src/components/IonRouterOutlet.ts#L292 We probably need to be more specific about which views we remount by taking into account the delta value that the app was navigated with. I will look into a fix. As a temporary workaround, you can downgrade to v5.6.13.

@whitersun
Copy link

whitersun commented Sep 28, 2021

Hi @liamdebeasi, I am afraid of if I downgrade to version 5.6.13 will influence the router.replace that got transition not remove from the prev view page rendered like the last issue of:
https://forum.ionicframework.com/t/page-transition-with-router-replace/214614/8

this was fixed that just not showing errors anymore. But that still had a router.replace problem, transition gets an incorrect view.
#22654

@whitersun
Copy link

whitersun commented Oct 4, 2021

Thanks for @liamdebeasi to point the problem, I finally solved this bug by changing abit code from

const mountIntermediaryViews = (outletId: number, enteringViewItem: ViewItem, leavingViewItem: ViewItem) => {

and below this link is my video record solution.
https://drive.google.com/file/d/1QCvTexQ5ZqYlSwzaE4aHbfXc6TDzuJAA/view?usp=sharing

Bug code:

  const mountIntermediaryViews = (outletId: number, enteringViewItem: ViewItem, leavingViewItem: ViewItem) => {
    const viewStack = viewStacks[outletId];
    if (!viewStack) return;

    const { enteringIndex: endIndex, leavingIndex: startIndex } = findViewIndex(viewStack, enteringViewItem, leavingViewItem);

    for (let i = startIndex + 1; i < endIndex; i++) {
      viewStack[i].mount = true;
    }
  }

fixed code:

  const mountIntermediaryViews = (outletId: number, enteringViewItem: ViewItem, leavingViewItem: ViewItem) => {
    const viewStack = viewStacks[outletId];
    if (!viewStack) return;

    const { enteringIndex: endIndex, leavingIndex: startIndex } = findViewIndex(viewStack, enteringViewItem, leavingViewItem);

    for (let i = startIndex; i < endIndex; i++) {
      return viewStack[i].mount = true;
    }
  }

or

  const mountIntermediaryViews = (outletId: number, enteringViewItem: ViewItem, leavingViewItem: ViewItem) => {
    const viewStack = viewStacks[outletId];
    if (!viewStack) return;

    const { enteringIndex: endIndex, leavingIndex: startIndex } = findViewIndex(viewStack, enteringViewItem, leavingViewItem);

    for (let i = startIndex + 1; i < endIndex; i++) {
      return viewStack[i - 1].mount = true;
    }
  }

@liamdebeasi liamdebeasi modified the milestones: 5.8.3, 5.8.4, 5.8.5 Oct 7, 2021
@liamdebeasi
Copy link
Contributor

Thanks for the issue. Can everyone try the following dev build and let me know if it resolves the issue?

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

@tigohenryschultz
Copy link
Contributor

tigohenryschultz commented Oct 11, 2021

Installed that build to test @liamdebeasi but I get this error now:

image

Old:
"@ionic/vue": "^5.8.2",
"@ionic/vue-router": "^5.8.2",

New:
"@ionic/vue": "^5.9.0-dev.202110111659.80a63d4",
"@ionic/vue-router": "^5.9.0-dev.202110111659.80a63d4",

@liamdebeasi
Copy link
Contributor

Do you have an example app I can test in?

@tigohenryschultz
Copy link
Contributor

Do you have an example app I can test in?

Unfortunately no this is our main app

@liamdebeasi
Copy link
Contributor

Can you reproduce the issue in an Ionic starter app? I cannot see this on my end, so it is hard to pinpoint what the issue may be.

@tigohenryschultz
Copy link
Contributor

Cleaning out my node modules and re-building to have a cleaner test

@tigohenryschultz
Copy link
Contributor

Can you reproduce the issue in an Ionic starter app? I cannot see this on my end, so it is hard to pinpoint what the issue may be.

Nuked my node modules and rebuilt, it is fine and I don't see the bug anymore, thanks @liamdebeasi sorry for false flag

@liamdebeasi
Copy link
Contributor

Glad the issue is resolved, and thanks for testing!

@liamdebeasi
Copy link
Contributor

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

Please feel free to continue testing the dev build, and let me know if you run into any issues. Thanks!

@tigohenryschultz
Copy link
Contributor

We had to revert 5.9.0-dev.202110121546.9dce03f back to 5.8.3 because the @ionChange event stopped working on ion-radio-group

@liamdebeasi
Copy link
Contributor

@tigohenryschultz Can you provide a reproduction?

@tigohenryschultz
Copy link
Contributor

Is there an ionic/vue boilerplate for jsfiddle.com I could point to the dev build for reproducing?

@liamdebeasi
Copy link
Contributor

We do not have a boilerplate JSFiddle at the moment. The easiest approach is probably to create a blank Ionic starter app, reproduce the issue there, and then push that app to GitHub.

@tigohenryschultz
Copy link
Contributor

https://github.com/tigohenryschultz/radio-button-group-bug

Select a radio option should trigger:

  console.log('radio changed to:', value);

5.8.4 does, the 5.9.0 doesnt

@liamdebeasi
Copy link
Contributor

Looks like that dev build had some other experimental work in it. Here's a new dev build with only the Vue fixes:

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

@tigohenryschultz
Copy link
Contributor

Awesome, thanks again @liamdebeasi

@ionitron-bot
Copy link

ionitron-bot bot commented Nov 22, 2021

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 Nov 22, 2021
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.

5 participants