feat(vue-router): upgrade to vue-router 5#31117
Conversation
…istory, and view stacks
…o feat/vue-router-upgrade
…o feat/vue-router-upgrade
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
561f950 to
3f69ec4
Compare
|
Nice, Will it work with |
@reslear Hey, thanks! We don't test |
thetaPC
left a comment
There was a problem hiding this comment.
LGTM, thank you!
All my comments are suggestions and not blockers. I found no issues during the manual testing.
| name: 'Mobile Chrome', | ||
| use: { | ||
| browserName: 'chromium', | ||
| ...devices['Pixel 5'], |
There was a problem hiding this comment.
Why is this one testing a phone while Angular tests a desktop?
There was a problem hiding this comment.
Swipe-to-go-back is iOS-only and only fires on a touch viewport, so the gesture tests need a phone device profile. The Angular suite doesn't exercise gestures, so a desktop chrome viewport is fine there.
There was a problem hiding this comment.
It also doesn't make a lot of sense to me that we'd test on non-mobile views primarily considering most of our use case is mobile views
There was a problem hiding this comment.
You're correct, even core has it as mobiles. Then the real question is why Angular uses Desktop? 🤔 @brandyscarney you added it, what was the reason? We should change it at a different time if we can.
There was a problem hiding this comment.
I don't remember 🤷♀️ It was probably just because it migrated from Cypress which tested on desktop.
brandyscarney
left a comment
There was a problem hiding this comment.
I found some issues while testing that I left as a comment on Jira. Great updates otherwise!
thetaPC
left a comment
There was a problem hiding this comment.
LGTM with minor suggestions
|
|
||
| ## Running the Test Suites | ||
|
|
||
| `packages/react-router/scripts/test_runner.sh` orchestrates the React Router test suites end to end: it builds `@ionic/core`, `@ionic/react`, and `@ionic/react-router`, builds the test app, syncs local packages, starts the dev server, and runs Cypress and Playwright in sequence. |
There was a problem hiding this comment.
Let's mention that it will run the latest version.
|
|
||
| ## Running the Test Suites | ||
|
|
||
| `packages/vue-router/scripts/test_runner.sh` orchestrates the Vue + Vue Router test suites end to end: it builds `@ionic/core`, `@ionic/vue`, and `@ionic/vue-router`, builds the test app, syncs local packages, runs Vitest unit tests in jsdom, then starts the Vite dev server and runs Cypress and Playwright e2e suites against it. |
There was a problem hiding this comment.
Let's mention that it will run the latest version.
| | `--playwright-only` | Run only the Playwright e2e suite | | ||
| | `--spec <pattern>` | Filter Playwright specs by file path | | ||
| | `--app <name>` | Pick a different app variant from `packages/react-router/test/apps/` (default: `reactrouter6-react18`) | | ||
| | `--serve` | Start the dev server only and open the browser | |
There was a problem hiding this comment.
Let's mention that it will run the latest version.
|
|
||
| ## Running the Test Suites | ||
|
|
||
| `packages/vue-router/scripts/test_runner.sh` orchestrates the Vue + Vue Router test suites end to end: it builds `@ionic/core`, `@ionic/vue`, and `@ionic/vue-router`, builds the test app, syncs local packages, runs Vitest unit tests in jsdom, then starts the Vite dev server and runs Cypress and Playwright e2e suites against it. |
There was a problem hiding this comment.
Let's mention that it will run the latest version.
| | `--spec <pattern>` | Filter Cypress specs by file path | | ||
| | `--pw-spec <pattern>` | Filter Playwright specs by file path | | ||
| | `--app <name>` | Pick a different app variant from `packages/vue/test/apps/` (default: `vue3`) | | ||
| | `--serve` | Start the dev server only and open the browser | |
There was a problem hiding this comment.
Let's mention that it will run the latest version.
There was a problem hiding this comment.
You mentioned that this fix is for a long-standing bug. Is there an issue associated to it? Maybe we should add the fix in main so the community can get it sooner.
There was a problem hiding this comment.
I think there's a card in the vue epic that might be related, if not I'll go looking to see if it's reported. I'll probably just fix it in main the same way and deal with the conflict next update from main
brandyscarney
left a comment
There was a problem hiding this comment.
Looks good! Thanks for fixing the issues I found. 🚀
…o feat/vue-router-upgrade
Issue number: internal
What is the current behavior?
@ionic/vue-routerand@ionic/vuebuild against vue-router 4What is the new behavior?
Bumps
vue-routerto^5.0.6andvueto^3.5.0(vue-router 5 raises its peer to^3.5.0). We also added Playwright tests for Vue router that are in full parity with the previous Jest and removed the Jest tests and replaced them with Playwright.This PR also makes the current Vue test app, which is also used for the Vue Router automated tests, get rebuilt with the current PR version for testing in the Vercel preview links.
Does this introduce a breaking change?
Consumer apps that pin
vue-routerthemselves need to upgrade to^5.0.0, and apps that explicitly pinvueneed to bump to^3.5.0Other information
CI changes: CI no longer runs
npm run test.spec(the script and Jest devDeps are gone), and now runs playwright testsPreview (Vue + Vue Router test app, demos both packages from this PR):
https://ionic-framework-git-feat-vue-router-upgrade-ionic1.vercel.app/vue/