Add config url trailing slash #4716
Conversation
following the opening of this issue (vuestorefront#4684) I created a configuration ( urlTrailingSlash ) to add or not the slash at the end of a url. I changed the normalizeUrlPath function so that depending on the configuration the slash is added or not. I also changed the prepareDynamicRoute function and added the pathToRegexpOptions attribute with the strict option following the slash config. pathToRegexpOptions tells the vuejs router not to remove the slash.
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/shopware-pwa/vue-storefront/9ffiioxho |
|
|
|
Hi, After some test there are still problems the code does not work for all cases. I try to update the PR quickly Thanjs |
|
@tdugue can we do that without config? 🤔 I think it should work by default, wdyt? |
|
@tdugue please rebase this branch to |
| {}, | ||
| userRoute, | ||
| routeData, | ||
| { path: normalizedPath, name: `urldispatcher-${normalizedPath}`, pathToRegexpOptions: { strict: config.seo.urlTrailingSlash } } |
There was a problem hiding this comment.
please remove this config and set strict to true. We want to use strict mode by default.
function prepareDynamicRoute (routeData: LocalizedRoute, path: string): RouteConfig {
const userRoute = RouterManager.findByName(routeData.name)
if (userRoute) {
const normalizedPath = `${path.startsWith('/') ? '' : '/'}${path}`
const dynamicRoute = Object.assign(
{},
userRoute,
routeData,
{
path: normalizedPath,
name: `urldispatcher-${normalizedPath}`,
pathToRegexpOptions: { strict: true }
}
)
return dynamicRoute
} else {
Logger.error('Route not found ' + routeData['name'], 'dispatcher')()
return null
}
}
| @@ -57,7 +62,8 @@ export function findRouteByPath (path: string): RouteConfig { | |||
| export function normalizeUrlPath (url: string): string { | |||
| if (url && url.length > 0) { | |||
| if (url.length > 0 && !url.startsWith('/')) url = `/${url}` | |||
| if (url.endsWith('/')) url = url.slice(0, -1) | |||
| if (url.endsWith('/') && !config.seo.urlTrailingSlash) url = url.slice(0, -1) | |||
| if (!url.endsWith('/') && config.seo.urlTrailingSlash) url += '/' | |||
There was a problem hiding this comment.
this needs to be parameterized, please change it to
export function normalizeUrlPath (url: string, clearTrailingSlash: boolean = true): string {
if (url && url.length > 0) {
if (url.length > 0 && !url.startsWith('/')) url = `/${url}`
if (url.endsWith('/') && clearTrailingSlash) url = url.slice(0, -1)
const queryPos = url.indexOf('?')
if (queryPos > 0) url = url.slice(0, queryPos)
}
return url
}
There was a problem hiding this comment.
this change probably require update in tests ☝️
| @@ -57,7 +62,8 @@ export function findRouteByPath (path: string): RouteConfig { | |||
| export function normalizeUrlPath (url: string): string { | |||
| if (url && url.length > 0) { | |||
| if (url.length > 0 && !url.startsWith('/')) url = `/${url}` | |||
| if (url.endsWith('/')) url = url.slice(0, -1) | |||
| if (url.endsWith('/') && !config.seo.urlTrailingSlash) url = url.slice(0, -1) | |||
| if (!url.endsWith('/') && config.seo.urlTrailingSlash) url += '/' | |||
| const queryPos = url.indexOf('?') | |||
| if (queryPos > 0) url = url.slice(0, queryPos) | |||
| } | |||
There was a problem hiding this comment.
Last changes in different file:
- https://github.com/DivanteLtd/vue-storefront/blob/master/core/modules/url/router/beforeEach.ts#L26
changeconst path = normalizeUrlPath(to.path)toconst path = normalizeUrlPath(to.path, false)=> in this place we removed trailing slash, so we need to usenormalizeUrlPathwithfalsewhich will result in keeping trailing slash - https://github.com/DivanteLtd/vue-storefront/blob/master/core/modules/url/router/beforeEach.ts#L33
changeif (routeData) {toif (routeData && !routeData.name.endsWith('page-not-found')) {
| @@ -92,6 +92,7 @@ | |||
| "seo": { | |||
| "useUrlDispatcher": true, | |||
| "disableUrlRoutesPersistentCache": true, | |||
| "urlTrailingSlash": false, | |||
|
Due to the author's inactivity, I decided to finish this PR on my own: #5372 |
Related Issues
following the opening of this issue (#4684) I created a configuration ( urlTrailingSlash ) to add or not the slash at the end of a url.
I changed the normalizeUrlPath function so that depending on the configuration the slash is added or not.
I also changed the prepareDynamicRoute function and added the pathToRegexpOptions attribute with the strict option following the slash config. pathToRegexpOptions tells the vuejs router not to remove the slash.
Short Description and Why It's Useful
Screenshots of Visual Changes before/after (if There Are Any)
Which Environment This Relates To
Check your case. In case of any doubts please read about Release Cycle
developbranch and want to merge it back todevelopreleasebranch and want to merge it back toreleasehotfixormasterbranch and want to merge it back tohotfixUpgrade Notes and Changelog
IMPORTANT NOTICE - Remember to update
CHANGELOG.mdwith description of your changeContribution and Currently Important Rules Acceptance