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

fix(vue-app, vue-renderer, utils): respect trailingSlash setting for payloads #8489

Merged
merged 6 commits into from
Dec 17, 2020

Conversation

danielroe
Copy link
Member

@danielroe danielroe commented Dec 14, 2020

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)

Description

This respects the route URL so payloads won't be refreshed when trailingSlash: true. Note this will still refresh payloads if the link doesn't respect the trailingSlash settings - there is still a need for a <NuxtLink> that respects the trailingSlash settings.

closes #8488
closes #8476

Checklist:

Sorry, something went wrong.

Verified

This commit was signed with the committer’s verified signature.
danielroe Daniel Roe
…*/`)

closes #8488

Verified

This commit was signed with the committer’s verified signature.
danielroe Daniel Roe
@codecov-io
Copy link

codecov-io commented Dec 14, 2020

Codecov Report

Merging #8489 (2895af8) into dev (fe02743) will decrease coverage by 0.01%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #8489      +/-   ##
==========================================
- Coverage   67.95%   67.93%   -0.02%     
==========================================
  Files          91       91              
  Lines        3916     3920       +4     
  Branches     1069     1071       +2     
==========================================
+ Hits         2661     2663       +2     
- Misses       1016     1017       +1     
- Partials      239      240       +1     
Flag Coverage Δ
unittests 67.93% <50.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/vue-renderer/src/renderers/ssr.js 0.00% <0.00%> (ø)
packages/utils/src/route.js 93.25% <100.00%> (+0.08%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe02743...2895af8. Read the comment docs.

pi0
pi0 previously approved these changes Dec 14, 2020
Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notes for later:

  • We can expose withoutQuery from ufo later on (or simply using new ponyfill .pathname
  • We may add tests for full-static to trailing-slash PR

Comment on lines +308 to +310
<% if (!nuxtOptions.router.trailingSlash) { %>
path = withoutTrailingSlash(path)
<% } %>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could normalise to no trailing slashes, which would also solve this problem, but would have the downside of breaking the distinction between parent/child routes (see #8423 - thoughts welcome @pi0).

@danielroe danielroe marked this pull request as ready for review December 14, 2020 15:26
pi0 and others added 3 commits December 16, 2020 14:41

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@pi0 pi0 merged commit fb191d2 into dev Dec 17, 2020
@pi0
Copy link
Member

pi0 commented Dec 17, 2020

Should be available shortly with nuxt-edge and landed with 2.15.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

catchall routes with trailingSlash: true lead to 404s when generated trailingSlash:true corrupts payloads
4 participants