-
-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
fix(vue-app): pass router mode to getLocation
#6658
Conversation
On the client, Nuxt creates "synthetic" route initially, before VueRouter has initialized so that plugins and middlewares have something to work with at that early stage already. The problem was that initial route was created without taking router's `mode` into consideration so it created route with normal path rather than hash-based path. That confused nuxt-i18n's logic that tries to redirect user to correct locale on initial navigation. As for tests: - created new `spa-hash` fixture config so that we can run tests in hash mode - new fixture is using files from `spa` fixture to avoid duplication - change `spa.test.js` testsuite to run same tests on both fixtures - disable `/тест雨 (test non ascii route)` test in hash mode as it is failing for unrelated reason (I haven't investigated deeper). - add `Initial route has correct fullPath` test to test the bug fixed here Resolves nuxt-modules/i18n#523
Much more pleasant to review after enabling |
Codecov Report
@@ Coverage Diff @@
## dev #6658 +/- ##
==========================================
- Coverage 95.83% 95.79% -0.04%
==========================================
Files 78 78
Lines 2713 2713
Branches 702 702
==========================================
- Hits 2600 2599 -1
- Misses 98 99 +1
Partials 15 15
Continue to review full report at Codecov.
|
return { window, head, html } | ||
} | ||
// Runs tests in specified router mode (either "hash" or "history"). | ||
function spaTests ({ isHashMode }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we extract the hash mode specific test cases into a separted file like spa.hash.test.js
for more readable and easier to maintain in future ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no "hash specific" testcases per se. All are valid testcases for both modes (apart from one test which fails in hash mode for some unknown reason). So it wouldn't make sense to create two identical sets of tests and two identical sets of files in a new fixture.
getLocation
Types of changes
Description
On the client, Nuxt creates "synthetic" route initially, before VueRouter
has initialized so that plugins and middlewares have something to work
with at that early stage already. The problem was that initial route
was created without taking router's
mode
into consideration so itcreated route with normal path rather than hash-based path. That confused
nuxt-i18n's logic that tries to redirect user to correct locale on
initial navigation.
As for tests:
spa-hash
fixture config so that we can run tests in hash modespa
fixture to avoid duplicationspa.test.js
testsuite to run same tests on both fixtures/тест雨 (test non ascii route)
test in hash mode as it is failingfor unrelated reason (I haven't investigated deeper).
Initial route has correct fullPath
test to test the bug fixed hereResolves nuxt-modules/i18n#523
Checklist: