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(utils): respect patterns within paths when sorting routes #20669

Merged
merged 31 commits into from May 22, 2023
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
f751fc8
fix: update sortRoutes to sort * with letters
marcelobotega May 4, 2023
5511cb1
fix: update sortRoutes regex to sort * with letters
marcelobotega May 4, 2023
f61e85f
test: add sort route fixture
marcelobotega May 4, 2023
603b826
fix: add a new level to taken the / into account
marcelobotega May 4, 2023
aab77d1
test: add fixture for route sort with slash
marcelobotega May 4, 2023
cab1595
test: update snapshot
marcelobotega May 4, 2023
aebec9f
Merge branch '2.x' into bugfix/sorte-route-fix
marcelobotega May 4, 2023
5470111
test: update snapshot
marcelobotega May 4, 2023
d896bc7
chore: update yarn.lock
marcelobotega May 4, 2023
4a14e41
chore: rename test file
marcelobotega May 4, 2023
4869e3e
test: remove file with * that is not allowed
marcelobotega May 4, 2023
d317d40
refactor: remove regex change and the new level on the ternary if
marcelobotega May 5, 2023
67b3b82
test: add posix to sortRoute unit test
marcelobotega May 5, 2023
9c7569d
refactor: update dynamic route regex
marcelobotega May 5, 2023
052cc53
refactor: remove fixtures and package nuxtjs/i18n in favor of unit tests
marcelobotega May 5, 2023
09a4616
test: add sortRoute unit tests
marcelobotega May 5, 2023
0872968
test: update windows snapshot
marcelobotega May 5, 2023
919287a
Merge branch '2.x' into bugfix/sorte-route-fix
marcelobotega May 8, 2023
2c64cfe
Merge branch '2.x' into bugfix/sorte-route-fix
marcelobotega May 9, 2023
db91d5d
Merge branch '2.x' into bugfix/sorte-route-fix
marcelobotega May 11, 2023
9c339ea
Merge branch '2.x' into bugfix/sorte-route-fix
marcelobotega May 15, 2023
dcbf3b7
Merge branch '2.x' into bugfix/sorte-route-fix
marcelobotega May 16, 2023
bce4af8
fix(utils): update route order check to take only * into account and …
marcelobotega May 16, 2023
125094b
fix: update regex to be more efficient
marcelobotega May 17, 2023
640529c
Merge branch '2.x' into bugfix/sorte-route-fix
marcelobotega May 17, 2023
3c12270
Merge remote-tracking branch 'origin/2.x' into bugfix/sorte-route-fix
danielroe May 19, 2023
696dbc6
Merge remote-tracking branch 'origin/2.x' into bugfix/sorte-route-fix
danielroe May 19, 2023
bde87d3
fix(utils): update getRoutePathExtension to check for _ in the end of…
marcelobotega May 22, 2023
d24275a
Merge branch '2.x' into bugfix/sorte-route-fix
marcelobotega May 22, 2023
431d315
revert(utils): update getRoutePathExtension to check for _ in the end…
marcelobotega May 22, 2023
bea8f4d
Merge branch '2.x' into bugfix/sorte-route-fix
marcelobotega May 22, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions packages/utils/src/route.js
Expand Up @@ -97,7 +97,7 @@ function cleanChildrenRoutes (routes, isChild = false, routeNameSplitter = '-',
return routes
}

const DYNAMIC_ROUTE_REGEX = /^\/([:*])/
const DYNAMIC_ROUTE_REGEX = /^\/?.*([:*])/
marcelobotega marked this conversation as resolved.
Show resolved Hide resolved

export const sortRoutes = function sortRoutes (routes) {
routes.sort((a, b) => {
Expand Down Expand Up @@ -126,13 +126,13 @@ export const sortRoutes = function sortRoutes (routes) {
if (res !== 0) {
break
}
y = _a[i] === '*' ? 2 : _a[i].includes(':') ? 1 : 0
z = _b[i] === '*' ? 2 : _b[i].includes(':') ? 1 : 0
y = _a[i] === '*' ? 3 : _a[i].includes(':') ? 2 : _a[i].includes('*') ? 1 : 0
z = _b[i] === '*' ? 3 : _b[i].includes(':') ? 2 : _b[i].includes('*') ? 1 : 0
res = y - z
// If a.length >= b.length
if (i === _b.length - 1 && res === 0) {
// unless * found sort by level, then alphabetically
res = _a[i] === '*'
res = _a[i].includes('*')
? -1
: (
_a.length === _b.length ? a.path.localeCompare(b.path) : (_a.length - _b.length)
Expand All @@ -142,7 +142,7 @@ export const sortRoutes = function sortRoutes (routes) {

if (res === 0) {
// unless * found sort by level, then alphabetically
res = _a[i - 1] === '*' && _b[i]
res = _a[i - 1].includes('*') && _b[i]
? 1
: (
_a.length === _b.length ? a.path.localeCompare(b.path) : (_a.length - _b.length)
Expand Down
144 changes: 117 additions & 27 deletions packages/utils/test/__snapshots__/route.test.js.snap
Expand Up @@ -26,6 +26,12 @@ exports[`util: route util: route create createRoutes should allow snake case rou
"name": "parent-child-test",
"path": "/parent/child/test",
},
{
"chunkName": "pages/index",
"component": "/some/nuxt/app/pages/index.vue",
"name": "index",
"path": "/",
},
{
"chunkName": "pages/another_route/_id",
"component": "/some/nuxt/app/pages/another_route/_id.vue",
Expand All @@ -38,12 +44,6 @@ exports[`util: route util: route create createRoutes should allow snake case rou
"name": "subpage-param",
"path": "/subpage/:param?",
},
{
"chunkName": "pages/index",
"component": "/some/nuxt/app/pages/index.vue",
"name": "index",
"path": "/",
},
{
"chunkName": "pages/_param",
"component": "/some/nuxt/app/pages/_param.vue",
Expand Down Expand Up @@ -79,6 +79,12 @@ exports[`util: route util: route create createRoutes should allow snake case rou
"name": "parent-child-test",
"path": "/parent/child/test",
},
{
"chunkName": "pages/index",
"component": "\\\\\\\\some\\\\nuxt\\\\app\\\\pages\\\\index.vue",
"name": "index",
"path": "/",
},
{
"chunkName": "pages/another_route/_id",
"component": "\\\\\\\\some\\\\nuxt\\\\app\\\\pages\\\\another_route\\\\_id.vue",
Expand All @@ -91,12 +97,6 @@ exports[`util: route util: route create createRoutes should allow snake case rou
"name": "subpage-param",
"path": "/subpage/:param?",
},
{
"chunkName": "pages/index",
"component": "\\\\\\\\some\\\\nuxt\\\\app\\\\pages\\\\index.vue",
"name": "index",
"path": "/",
},
{
"chunkName": "pages/_param",
"component": "\\\\\\\\some\\\\nuxt\\\\app\\\\pages\\\\_param.vue",
Expand Down Expand Up @@ -144,6 +144,15 @@ exports[`util: route util: route create createRoutes should enforce trailing sla
"strict": true,
},
},
{
"chunkName": "pages/index",
"component": "/some/nuxt/app/pages/index.vue",
"name": "index",
"path": "/",
"pathToRegexpOptions": {
"strict": true,
},
},
{
"chunkName": "pages/another_route/_id",
"component": "/some/nuxt/app/pages/another_route/_id.vue",
Expand All @@ -162,15 +171,6 @@ exports[`util: route util: route create createRoutes should enforce trailing sla
"strict": true,
},
},
{
"chunkName": "pages/index",
"component": "/some/nuxt/app/pages/index.vue",
"name": "index",
"path": "/",
"pathToRegexpOptions": {
"strict": true,
},
},
{
"chunkName": "pages/_param",
"component": "/some/nuxt/app/pages/_param.vue",
Expand Down Expand Up @@ -221,6 +221,15 @@ exports[`util: route util: route create createRoutes should remove trailing slas
"strict": true,
},
},
{
"chunkName": "pages/index",
"component": "/some/nuxt/app/pages/index.vue",
"name": "index",
"path": "/",
"pathToRegexpOptions": {
"strict": true,
},
},
{
"chunkName": "pages/another_route/_id",
"component": "/some/nuxt/app/pages/another_route/_id.vue",
Expand All @@ -239,23 +248,104 @@ exports[`util: route util: route create createRoutes should remove trailing slas
"strict": true,
},
},
{
"chunkName": "pages/_param",
"component": "/some/nuxt/app/pages/_param.vue",
"name": "param",
"path": "/:param",
"pathToRegexpOptions": {
"strict": true,
},
},
]
`;

exports[`util: route util: route sortRoutes sortRoutes should sort routes 1`] = `
[
{
"chunkName": "pages/de/index",
"component": "/some/nuxt/app/pages/de/index.vue",
"name": "de",
"path": "/de",
},
{
"chunkName": "pages/parent/index",
"component": "/some/nuxt/app/pages/parent/index.vue",
"name": "parent",
"path": "/parent",
},
{
"chunkName": "pages/snake_case_route",
"component": "/some/nuxt/app/pages/snake_case_route.vue",
"name": "snake_case_route",
"path": "/snake_case_route",
},
{
"chunkName": "pages/parent/child/index",
"component": "/some/nuxt/app/pages/parent/child/index.vue",
"name": "parent-child",
"path": "/parent/child",
},
{
"chunkName": "pages/parent/child/test",
"component": "/some/nuxt/app/pages/parent/child/test.vue",
"name": "parent-child-test",
"path": "/parent/child/test",
},
{
"chunkName": "pages/index",
"component": "/some/nuxt/app/pages/index.vue",
"name": "index",
"path": "/",
"pathToRegexpOptions": {
"strict": true,
},
},
{
"children": [
{
"chunkName": "pages/another_route/rout*",
"component": "/some/nuxt/app/pages/another_route/rout*.vue",
"name": "another_route-rout*",
"path": "",
},
],
"chunkName": "pages/another_route/rout*",
"component": "/some/nuxt/app/pages/another_route/rout*.vue",
"path": "/another_route/rout*",
},
{
"chunkName": "pages/another_route/_id",
"component": "/some/nuxt/app/pages/another_route/_id.vue",
"name": "another_route-id",
"path": "/another_route/:id?",
},
{
"chunkName": "pages/subpage/_param",
"component": "/some/nuxt/app/pages/subpage/_param.vue",
"name": "subpage-param",
"path": "/subpage/:param?",
},
{
"chunkName": "pages/parent/*",
"component": "/some/nuxt/app/pages/parent/*.vue",
"name": "parent-*",
"path": "/parent/*",
},
{
"chunkName": "pages/de*",
"component": "/some/nuxt/app/pages/de*.vue",
"name": "de*",
"path": "/de*",
},
{
"chunkName": "pages/_param",
"component": "/some/nuxt/app/pages/_param.vue",
"name": "param",
"path": "/:param",
"pathToRegexpOptions": {
"strict": true,
},
},
{
"chunkName": "pages/*",
"component": "/some/nuxt/app/pages/*.vue",
"name": "*",
"path": "/*",
},
]
`;
107 changes: 106 additions & 1 deletion packages/utils/test/route.test.js
@@ -1,4 +1,4 @@
import { flatRoutes, createRoutes, guardDir, promisifyRoute } from '../src/route'
import { flatRoutes, createRoutes, guardDir, promisifyRoute, sortRoutes } from '../src/route'

describe('util: route', () => {
test('should flat route with path', () => {
Expand Down Expand Up @@ -237,4 +237,109 @@ describe('util: route', () => {
expect(routesResult).toMatchSnapshot()
})
})

describe('util: route sortRoutes', () => {
const files = [
'pages/_param.vue',
'pages/de/index.vue',
'pages/index.vue',
'pages/*.vue',
'pages/another_route/rout*.vue',
marcelobotega marked this conversation as resolved.
Show resolved Hide resolved
'pages/snake_case_route.vue',
'pages/subpage/_param.vue',
'pages/de*.vue',
'pages/parent/index.vue',
'pages/parent/*.vue',
'pages/another_route/_id.vue',
'pages/another_route/rout*.vue',
'pages/parent/child/index.vue',
'pages/parent/child/test.vue'
]
const srcDir = '/some/nuxt/app'
const pagesDir = 'pages'

test.posix('sortRoutes should sort routes', () => {
const routesResult = createRoutes({ files, srcDir, pagesDir })
expect(routesResult).toMatchSnapshot()
})
test('Should sortRoutes with extendRoutes using *', () => {
const routes = [
{ path: '/poetry' },
{ path: '/reports' },
{ path: '*' },
{ path: '/de/about' },
{ path: '/' },
{ path: '/about' },
{ path: '/de' },
{ path: '/tech' },
{ path: '/de/tech' },
{ path: '/de*' },
{ path: '/:post' },
{ path: '/de/:post' },
{ path: '/de/reports' },
{ path: '/de/poetry' }
]

sortRoutes(routes)

expect(routes).toEqual(
[
{ path: '/about' },
{ path: '/de' },
{ path: '/poetry' },
{ path: '/reports' },
{ path: '/tech' },
{ path: '/de/about' },
{ path: '/de/poetry' },
{ path: '/de/reports' },
{ path: '/de/tech' },
{ path: '/' },
{ path: '/de/:post' },
{ path: '/de*' },
{ path: '/:post' },
{ path: '*' }
]
)
})

test('Should sortRoutes with extendRoutes using /*', () => {
const routes = [
{ path: '/poetry' },
{ path: '/reports' },
{ path: '/*' },
{ path: '/de/about' },
{ path: '/about' },
{ path: '/de' },
{ path: '/tech' },
{ path: '/de/tech' },
{ path: '/de/*' },
{ path: '/' },
{ path: '/:post' },
{ path: '/de/:post' },
{ path: '/de/reports' },
{ path: '/de/poetry' }
]

sortRoutes(routes)

expect(routes).toEqual(
[
{ path: '/about' },
{ path: '/de' },
{ path: '/poetry' },
{ path: '/reports' },
{ path: '/tech' },
{ path: '/de/about' },
{ path: '/de/poetry' },
{ path: '/de/reports' },
{ path: '/de/tech' },
{ path: '/' },
{ path: '/de/:post' },
{ path: '/de/*' },
{ path: '/:post' },
{ path: '/*' }
]
)
})
})
})
26 changes: 13 additions & 13 deletions test/dev/dynamic-routes.test.js
Expand Up @@ -54,23 +54,23 @@ describe('dynamic routes', () => {
// pages/test/songs/toto.vue
expect(routes[5].path).toBe('/test/songs/toto')
expect(routes[5].name).toBe('test-songs-toto')

// pages/index.vue
expect(routes[6].path).toBe('/')
expect(routes[6].name).toBe('index')

// pages/test/projects/_category.vue
expect(routes[6].path).toBe('/test/projects/:category')
expect(routes[6].name).toBe('test-projects-category')
expect(routes[7].path).toBe('/test/projects/:category')
expect(routes[7].name).toBe('test-projects-category')
// pages/test/songs/_id.vue
expect(routes[7].path).toBe('/test/songs/:id?')
expect(routes[7].name).toBe('test-songs-id')
expect(routes[8].path).toBe('/test/songs/:id?')
expect(routes[8].name).toBe('test-songs-id')
// pages/users/_id.vue
expect(routes[8].path).toBe('/users/:id?')
expect(routes[8].name).toBe('users-id')
expect(routes[9].path).toBe('/users/:id?')
expect(routes[9].name).toBe('users-id')
// pages/test/_.vue
expect(routes[9].path).toBe('/test/*')
expect(routes[9].name).toBe('test-all')

// pages/index.vue
expect(routes[10].path).toBe('/')
expect(routes[10].name).toBe('index')

expect(routes[10].path).toBe('/test/*')
expect(routes[10].name).toBe('test-all')
// pages/_slug.vue
expect(routes[11].path).toBe('/:slug')
expect(routes[11].name).toBe('slug')
Expand Down