From a3d31061c8d59eafaafe3936b6d9b5502863df8a Mon Sep 17 00:00:00 2001 From: Daniel Rios Pavia Date: Tue, 29 Nov 2022 14:37:55 +0100 Subject: [PATCH 1/6] feat: transform optional routes from remix to react router --- .../__tests__/routesConvention-test.ts | 41 +++++++++++++++++++ packages/remix-dev/config/routesConvention.ts | 15 ++++++- 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/packages/remix-dev/__tests__/routesConvention-test.ts b/packages/remix-dev/__tests__/routesConvention-test.ts index 1497246ce16..3dbfbe5071f 100644 --- a/packages/remix-dev/__tests__/routesConvention-test.ts +++ b/packages/remix-dev/__tests__/routesConvention-test.ts @@ -36,6 +36,47 @@ describe("createRoutePath", () => { ["[index]", "index"], ["test/inde[x]", "test/index"], ["[i]ndex/[[].[[]]", "index/[/[]"], + + // Optional segment routes + ["(routes)/$", "routes?/*"], + ["(routes)/($)", "routes?/*?"], // TODO: Fails, do we want to allow this? + ["(routes)/(sub)/$", "routes?/sub?/*"], + ["(routes).(sub)/$", "routes?/sub?/*"], + ["(routes.sub)/$", "routes?/sub?/*"], // TODO: Fails, do we want to allow this? + ["(routes)/($slug)", "routes?/:slug?"], + ["(routes)/sub/($slug)", "routes?/sub/:slug?"], + ["(routes).sub/($slug)", "routes?/sub/:slug?"], + ["($)", "*?"], // TODO: Fails, do we want to allow this? + ["(nested)/$", "nested?/*"], + ["(flat).$", "flat?/*"], + ["($slug)", ":slug?"], + ["(nested)/($slug)", "nested?/:slug?"], + ["(flat).($slug)", "flat?/:slug?"], + ["flat.(sub)", "flat/sub?"], + ["(nested)/(index)", "nested?"], // Fails with `"flat?/index?"` + ["(flat).(index)", "flat?"], // Fails with `"flat?/index?"` + ["(index)", undefined], // Fails with `"index?"`" + ["(__layout)/(index)", undefined], // Fails with __layout?/index? + ["__layout/(test)", "test?"], + ["__layout.(test)", "test?"], + ["__layout/($slug)", ":slug?"], + ["(nested)/__layout/($slug)", "nested?/:slug?"], + ["($slug[.]json)", ":slug.json?"], + ["(sub)/([sitemap.xml])", "sub?/sitemap.xml?"], + ["(sub)/[(sitemap.xml)]", "sub?/sitemap.xml?"], // TODO should this have been sub?/(sitemap.xml) + ["(posts)/($slug)/([image.jpg])", "posts?/:slug?/image.jpg?"], + [ + "($[$dollabills]).([.]lol)[/](what)/([$]).($)", // TODO outputs ":$dollabills?/.lol?/what?/$?/:?" + ":$dollabills/.lol/what/$/*", + ], + ["(sub).([[])", "sub?/[?"], + ["(sub).(])", "sub?/)?"], + ["(sub).([([])])", "sub/[]"], // Fails with "sub?/([)]?" + ["(sub).([[])", "sub?/[?"], + ["(beef])", "beef]?"], + ["([index])", "index?"], + ["(test)/(inde[x])", "test?/index?"], + ["([i]ndex)/([[]).([[]])", "index?/[?/[]?"], ]; for (let [input, expected] of tests) { diff --git a/packages/remix-dev/config/routesConvention.ts b/packages/remix-dev/config/routesConvention.ts index 4fb5fc3211e..e9f5ce22bbe 100644 --- a/packages/remix-dev/config/routesConvention.ts +++ b/packages/remix-dev/config/routesConvention.ts @@ -190,7 +190,20 @@ export function createRoutePath(partialRouteId: string): string | undefined { result = result.replace(/\/?index$/, ""); } - return result || undefined; + return generateOptionalDynamicRoutes(result) || undefined; +} + +function generateOptionalDynamicRoutes(routeId: string) { + let segments = routeId.split("/"); + + let newSegments = []; + for (let segment of segments) { + console.log(segment, "segment"); + console.log(segment.replace(/^\((.+)\)$/, "$1")); + newSegments.push(segment.replace(/^\((.+)\)$/, "$1?")); + } + + return newSegments.join("/"); } function findParentRouteId( From 109ad1326b8670608ae2905cb82027591285c9ce Mon Sep 17 00:00:00 2001 From: Daniel Rios Pavia Date: Tue, 29 Nov 2022 14:51:57 +0100 Subject: [PATCH 2/6] Add to contributors --- contributors.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/contributors.yml b/contributors.yml index bc025058fe8..1227cd93c7d 100644 --- a/contributors.yml +++ b/contributors.yml @@ -253,6 +253,7 @@ - lili21 - lionotm - liranm +- lordofthecactus - lpsinger - lswest - lucasdibz From af0a5c5e234816d48bdb222a013ce248da361895 Mon Sep 17 00:00:00 2001 From: Daniel Rios Pavia Date: Tue, 29 Nov 2022 14:58:44 +0100 Subject: [PATCH 3/6] Add changeset --- .changeset/moody-pants-own.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 .changeset/moody-pants-own.md diff --git a/.changeset/moody-pants-own.md b/.changeset/moody-pants-own.md new file mode 100644 index 00000000000..7b9579963cd --- /dev/null +++ b/.changeset/moody-pants-own.md @@ -0,0 +1,25 @@ +--- +"@remix-run/dev": minor +--- + +feat: remix optional segments + +Allows for the creation of optional route segments by using parenthesis. For example: +Creating the following file routes in remix `/($lang)/about` +this will match the following routes +``` +/en/about +/fr/about +/about +``` + +helpful for optional language paths. + +Another example `/(one)/($two)/(three).($four)` file routing would match +``` +/ +/one +/one/param1 +/one/param1/three +/one/param1/three/param2 +``` From 4fa421fdd7be45c7d04928b8468fdbe78cb6d041 Mon Sep 17 00:00:00 2001 From: Daniel Rios Pavia Date: Thu, 1 Dec 2022 00:31:06 +0100 Subject: [PATCH 4/6] fix(optional-segments): fix escaping of parenthesis --- .../__tests__/routesConvention-test.ts | 39 ++++++--- packages/remix-dev/config/routesConvention.ts | 79 +++++++++++++++---- 2 files changed, 92 insertions(+), 26 deletions(-) diff --git a/packages/remix-dev/__tests__/routesConvention-test.ts b/packages/remix-dev/__tests__/routesConvention-test.ts index 3dbfbe5071f..7114124bbcb 100644 --- a/packages/remix-dev/__tests__/routesConvention-test.ts +++ b/packages/remix-dev/__tests__/routesConvention-test.ts @@ -39,39 +39,36 @@ describe("createRoutePath", () => { // Optional segment routes ["(routes)/$", "routes?/*"], - ["(routes)/($)", "routes?/*?"], // TODO: Fails, do we want to allow this? ["(routes)/(sub)/$", "routes?/sub?/*"], ["(routes).(sub)/$", "routes?/sub?/*"], - ["(routes.sub)/$", "routes?/sub?/*"], // TODO: Fails, do we want to allow this? ["(routes)/($slug)", "routes?/:slug?"], ["(routes)/sub/($slug)", "routes?/sub/:slug?"], ["(routes).sub/($slug)", "routes?/sub/:slug?"], - ["($)", "*?"], // TODO: Fails, do we want to allow this? ["(nested)/$", "nested?/*"], ["(flat).$", "flat?/*"], ["($slug)", ":slug?"], ["(nested)/($slug)", "nested?/:slug?"], ["(flat).($slug)", "flat?/:slug?"], ["flat.(sub)", "flat/sub?"], - ["(nested)/(index)", "nested?"], // Fails with `"flat?/index?"` - ["(flat).(index)", "flat?"], // Fails with `"flat?/index?"` - ["(index)", undefined], // Fails with `"index?"`" - ["(__layout)/(index)", undefined], // Fails with __layout?/index? ["__layout/(test)", "test?"], ["__layout.(test)", "test?"], ["__layout/($slug)", ":slug?"], ["(nested)/__layout/($slug)", "nested?/:slug?"], ["($slug[.]json)", ":slug.json?"], ["(sub)/([sitemap.xml])", "sub?/sitemap.xml?"], - ["(sub)/[(sitemap.xml)]", "sub?/sitemap.xml?"], // TODO should this have been sub?/(sitemap.xml) + ["(sub)/[(sitemap.xml)]", "sub?/(sitemap.xml)"], ["(posts)/($slug)/([image.jpg])", "posts?/:slug?/image.jpg?"], [ - "($[$dollabills]).([.]lol)[/](what)/([$]).($)", // TODO outputs ":$dollabills?/.lol?/what?/$?/:?" - ":$dollabills/.lol/what/$/*", + "($[$dollabills]).([.]lol)[/](what)/([$]).$", + ":$dollabills?/.lol)/(what?/$?/*", + ], + [ + "($[$dollabills]).([.]lol)/(what)/([$]).($up)", + ":$dollabills?/.lol?/what?/$?/:up?", ], ["(sub).([[])", "sub?/[?"], - ["(sub).(])", "sub?/)?"], - ["(sub).([([])])", "sub/[]"], // Fails with "sub?/([)]?" + ["(sub).(])", "sub?/]?"], + ["(sub).([[]])", "sub?/[]?"], ["(sub).([[])", "sub?/[?"], ["(beef])", "beef]?"], ["([index])", "index?"], @@ -84,5 +81,23 @@ describe("createRoutePath", () => { expect(createRoutePath(input)).toBe(expected); }); } + + describe("optional segments", () => { + it("will only work when starting and ending a segment with parenthesis", () => { + let [input, expected] = ["(routes.sub)/$", "(routes/sub)/*"]; + expect(createRoutePath(input)).toBe(expected); + }); + + it("throws error on optional to splat routes", () => { + expect(() => createRoutePath("(routes)/($)")).toThrow("Splat"); + expect(() => createRoutePath("($)")).toThrow("Splat"); + }); + + it("throws errors on optional index with brackets routes", () => { + expect(() => createRoutePath("(nested)/(index)")).toThrow("index"); + expect(() => createRoutePath("(flat).(index)")).toThrow("index"); + expect(() => createRoutePath("(index)")).toThrow("index"); + }); + }); }); }); diff --git a/packages/remix-dev/config/routesConvention.ts b/packages/remix-dev/config/routesConvention.ts index e9f5ce22bbe..f16fb55cd37 100644 --- a/packages/remix-dev/config/routesConvention.ts +++ b/packages/remix-dev/config/routesConvention.ts @@ -112,12 +112,17 @@ export function defineConventionalRoutes( let escapeStart = "["; let escapeEnd = "]"; +let optionalStart = "("; +let optionalEnd = ")"; + // TODO: Cleanup and write some tests for this function export function createRoutePath(partialRouteId: string): string | undefined { let result = ""; let rawSegmentBuffer = ""; let inEscapeSequence = 0; + let inOptionalSegment = 0; + let optionalSegmentIndex = null; let skipSegment = false; for (let i = 0; i < partialRouteId.length; i++) { let char = partialRouteId.charAt(i); @@ -139,8 +144,34 @@ export function createRoutePath(partialRouteId: string): string | undefined { return char === "_" && nextChar === "_" && !rawSegmentBuffer; } + function isSegmentSeparator(checkChar = char) { + return ( + checkChar === "/" || checkChar === "." || checkChar === path.win32.sep + ); + } + + function isNewOptionalSegment() { + return ( + char === optionalStart && + lastChar !== optionalStart && + (isSegmentSeparator(lastChar) || lastChar === undefined) && + !inOptionalSegment && + !inEscapeSequence + ); + } + + function isCloseOptionalSegment(charCheck: String = char) { + return ( + charCheck === optionalEnd && + nextChar !== optionalEnd && + (isSegmentSeparator(nextChar) || nextChar === undefined) && + inOptionalSegment && + !inEscapeSequence + ); + } + if (skipSegment) { - if (char === "/" || char === "." || char === path.win32.sep) { + if (isSegmentSeparator()) { skipSegment = false; } continue; @@ -156,18 +187,40 @@ export function createRoutePath(partialRouteId: string): string | undefined { continue; } + if (isNewOptionalSegment()) { + inOptionalSegment++; + optionalSegmentIndex = result.length; + result += "("; + continue; + } + + if (isCloseOptionalSegment()) { + if (optionalSegmentIndex !== null) { + result = + result.slice(0, optionalSegmentIndex) + + result.slice(optionalSegmentIndex + 1); + } + optionalSegmentIndex = null; + inOptionalSegment--; + result += "?"; + continue; + } + if (inEscapeSequence) { result += char; continue; } - if (char === "/" || char === path.win32.sep || char === ".") { + if (isSegmentSeparator()) { if (rawSegmentBuffer === "index" && result.endsWith("index")) { result = result.replace(/\/?index$/, ""); } else { result += "/"; } + rawSegmentBuffer = ""; + inOptionalSegment = 0; + optionalSegmentIndex = null; continue; } @@ -179,6 +232,11 @@ export function createRoutePath(partialRouteId: string): string | undefined { rawSegmentBuffer += char; if (char === "$") { + if (nextChar === ")") { + throw new Error( + `Invalid route path: ${partialRouteId}. Splat route $ is already optional` + ); + } result += typeof nextChar === "undefined" ? "*" : ":"; continue; } @@ -190,20 +248,13 @@ export function createRoutePath(partialRouteId: string): string | undefined { result = result.replace(/\/?index$/, ""); } - return generateOptionalDynamicRoutes(result) || undefined; -} - -function generateOptionalDynamicRoutes(routeId: string) { - let segments = routeId.split("/"); - - let newSegments = []; - for (let segment of segments) { - console.log(segment, "segment"); - console.log(segment.replace(/^\((.+)\)$/, "$1")); - newSegments.push(segment.replace(/^\((.+)\)$/, "$1?")); + if (rawSegmentBuffer === "index" && result.endsWith("index?")) { + throw new Error( + `Invalid route path: ${partialRouteId}. Make index route optional by using [index]` + ); } - return newSegments.join("/"); + return result || undefined; } function findParentRouteId( From a356be442cac09162447ed0f6f62bd81715c1dcd Mon Sep 17 00:00:00 2001 From: Daniel Rios Pavia Date: Thu, 1 Dec 2022 01:04:45 +0100 Subject: [PATCH 5/6] small function fix --- packages/remix-dev/config/routesConvention.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/remix-dev/config/routesConvention.ts b/packages/remix-dev/config/routesConvention.ts index f16fb55cd37..851d3c62f1b 100644 --- a/packages/remix-dev/config/routesConvention.ts +++ b/packages/remix-dev/config/routesConvention.ts @@ -160,9 +160,9 @@ export function createRoutePath(partialRouteId: string): string | undefined { ); } - function isCloseOptionalSegment(charCheck: String = char) { + function isCloseOptionalSegment() { return ( - charCheck === optionalEnd && + char === optionalEnd && nextChar !== optionalEnd && (isSegmentSeparator(nextChar) || nextChar === undefined) && inOptionalSegment && From 85801f12620077e0ff8d482297eb9e082bc2edb9 Mon Sep 17 00:00:00 2001 From: Daniel Rios Date: Tue, 6 Dec 2022 19:29:46 +0100 Subject: [PATCH 6/6] Update packages/remix-dev/__tests__/routesConvention-test.ts Co-authored-by: Pedro Cattori --- packages/remix-dev/__tests__/routesConvention-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/remix-dev/__tests__/routesConvention-test.ts b/packages/remix-dev/__tests__/routesConvention-test.ts index 7114124bbcb..9ea2fdac07b 100644 --- a/packages/remix-dev/__tests__/routesConvention-test.ts +++ b/packages/remix-dev/__tests__/routesConvention-test.ts @@ -93,7 +93,7 @@ describe("createRoutePath", () => { expect(() => createRoutePath("($)")).toThrow("Splat"); }); - it("throws errors on optional index with brackets routes", () => { + it("throws errors on optional index without brackets routes", () => { expect(() => createRoutePath("(nested)/(index)")).toThrow("index"); expect(() => createRoutePath("(flat).(index)")).toThrow("index"); expect(() => createRoutePath("(index)")).toThrow("index");