diff --git a/integration/conventional-routes-test.ts b/integration/conventional-routes-test.ts index ff399894318..665ad3516c1 100644 --- a/integration/conventional-routes-test.ts +++ b/integration/conventional-routes-test.ts @@ -54,6 +54,23 @@ test.beforeAll(async () => { return

Foo

; } `, + "app/routes/nested/__pathless2.jsx": js` + import { Outlet } from "@remix-run/react"; + + export default function Layout() { + return ( + <> +
Pathless 2 Layout
+ + + ); + } + `, + "app/routes/nested/__pathless2/bar.jsx": js` + export default function Bar() { + return

Bar

; + } + `, }, }); @@ -102,4 +119,14 @@ function runTests() { expect(await app.getHtml()).toMatch("Foo"); expect(await app.getHtml()).toMatch("Number of matches: 3"); }); + + // This also asserts that we support multiple sibling pathless route layouts + test("displays page inside of second pathless layout", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/nested/bar"); + expect(await app.getHtml()).not.toMatch("Index"); + expect(await app.getHtml()).toMatch("Pathless 2 Layout"); + expect(await app.getHtml()).toMatch("Bar"); + expect(await app.getHtml()).toMatch("Number of matches: 3"); + }); } diff --git a/integration/route-collisions-test.ts b/integration/route-collisions-test.ts new file mode 100644 index 00000000000..a8c852ccd4b --- /dev/null +++ b/integration/route-collisions-test.ts @@ -0,0 +1,164 @@ +import { test, expect } from "@playwright/test"; + +import { createFixture, js } from "./helpers/create-fixture"; + +let ROOT_FILE_CONTENTS = js` + import { Outlet, Scripts } from "@remix-run/react"; + + export default function App() { + return ( + + + + + + + ); + } +`; + +let LAYOUT_FILE_CONTENTS = js` + import { Outlet } from "@remix-run/react"; + + export default function Layout() { + return + } +`; + +let LEAF_FILE_CONTENTS = js` + export default function Foo() { + return

Foo

; + } +`; + +test.describe("build failures", () => { + let errorLogs: string[]; + let oldConsoleError: typeof console.error; + + test.beforeEach(() => { + errorLogs = []; + oldConsoleError = console.error; + console.error = (str) => errorLogs.push(str); + }); + + test.afterEach(() => { + console.error = oldConsoleError; + }); + + test("detects path collisions inside pathless layout routes", async () => { + try { + await createFixture({ + files: { + "app/root.tsx": ROOT_FILE_CONTENTS, + "app/routes/foo.jsx": LEAF_FILE_CONTENTS, + "app/routes/__pathless.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/__pathless/foo.jsx": LEAF_FILE_CONTENTS, + }, + }); + expect(false).toBe(true); + } catch (e) { + expect(errorLogs[0]).toMatch( + 'Error: Path "foo" defined by route "routes/foo" conflicts with route "routes/__pathless/foo"' + ); + expect(errorLogs.length).toBe(1); + } + }); + + test("detects path collisions across pathless layout routes", async () => { + try { + await createFixture({ + files: { + "app/root.tsx": ROOT_FILE_CONTENTS, + "app/routes/__pathless.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/__pathless/foo.jsx": LEAF_FILE_CONTENTS, + "app/routes/__pathless2.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/__pathless2/foo.jsx": LEAF_FILE_CONTENTS, + }, + }); + expect(false).toBe(true); + } catch (e) { + expect(errorLogs[0]).toMatch( + 'Error: Path "foo" defined by route "routes/__pathless/foo" conflicts with route "routes/__pathless2/foo"' + ); + expect(errorLogs.length).toBe(1); + } + }); + + test("detects path collisions inside multiple pathless layout routes", async () => { + try { + await createFixture({ + files: { + "app/root.tsx": ROOT_FILE_CONTENTS, + "app/routes/foo.jsx": LEAF_FILE_CONTENTS, + "app/routes/__pathless.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/__pathless/__again.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/__pathless/__again/foo.jsx": LEAF_FILE_CONTENTS, + }, + }); + expect(false).toBe(true); + } catch (e) { + expect(errorLogs[0]).toMatch( + 'Error: Path "foo" defined by route "routes/foo" conflicts with route "routes/__pathless/__again/foo"' + ); + expect(errorLogs.length).toBe(1); + } + }); + + test("detects path collisions of index files inside pathless layouts", async () => { + try { + await createFixture({ + files: { + "app/root.tsx": ROOT_FILE_CONTENTS, + "app/routes/index.jsx": LEAF_FILE_CONTENTS, + "app/routes/__pathless.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/__pathless/index.jsx": LEAF_FILE_CONTENTS, + }, + }); + expect(false).toBe(true); + } catch (e) { + expect(errorLogs[0]).toMatch( + 'Error: Path "/" defined by route "routes/index" conflicts with route "routes/__pathless/index"' + ); + expect(errorLogs.length).toBe(1); + } + }); + + test("detects path collisions of index files across multiple pathless layouts", async () => { + try { + await createFixture({ + files: { + "app/root.tsx": ROOT_FILE_CONTENTS, + "app/routes/nested/__pathless.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/nested/__pathless/index.jsx": LEAF_FILE_CONTENTS, + "app/routes/nested/__oops.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/nested/__oops/index.jsx": LEAF_FILE_CONTENTS, + }, + }); + expect(false).toBe(true); + } catch (e) { + expect(errorLogs[0]).toMatch( + 'Error: Path "nested" defined by route "routes/nested/__oops/index" conflicts with route "routes/nested/__pathless/index"' + ); + expect(errorLogs.length).toBe(1); + } + }); + + test("detects path collisions of param routes inside pathless layouts", async () => { + try { + await createFixture({ + files: { + "app/root.tsx": ROOT_FILE_CONTENTS, + "app/routes/$param.jsx": LEAF_FILE_CONTENTS, + "app/routes/__pathless.jsx": LAYOUT_FILE_CONTENTS, + "app/routes/__pathless/$param.jsx": LEAF_FILE_CONTENTS, + }, + }); + expect(false).toBe(true); + } catch (e) { + expect(errorLogs[0]).toMatch( + 'Error: Path ":param" defined by route "routes/$param" conflicts with route "routes/__pathless/$param"' + ); + expect(errorLogs.length).toBe(1); + } + }); +}); diff --git a/packages/remix-dev/config/routesConvention.ts b/packages/remix-dev/config/routesConvention.ts index 41ee3775bbf..100215e099f 100644 --- a/packages/remix-dev/config/routesConvention.ts +++ b/packages/remix-dev/config/routesConvention.ts @@ -73,11 +73,63 @@ export function defineConventionalRoutes( let isIndexRoute = routeId.endsWith("/index"); let fullPath = createRoutePath(routeId.slice("routes".length + 1)); let uniqueRouteId = (fullPath || "") + (isIndexRoute ? "?index" : ""); - - if (uniqueRouteId) { + let isPathlessLayoutRoute = + routeId.split("/").pop()?.startsWith("__") === true; + + /** + * We do not try to detect path collisions for pathless layout route + * files because, by definition, they create the potential for route + * collisions _at that level in the tree_. For example, consider the + * following route structure: + * + * routes/ + * parent.tsx + * parent/ + * __pathless.tsx + * + * The route path for both parent.tsx and parent/__pathless.tsx + * is the same (/parent), but it's not expected you are matching at that + * level. It's up to you to handle that in your `parent.tsx` or provide + * an index route in your pathless route folder. + * + * Consider this more complex example where a user may want multiple + * pathless layout routes for different subfolders + * + * routes/ + * account.tsx + * account/ + * __public/ + * login.tsx + * perks.tsx + * __private/ + * orders.tsx + * profile.tsx + * __public.tsx + * __private.tsx + * + * In order to support both a public and private layout for `/account/*` + * URLs, we are creating a mutually exclusive set of URLs beneath 2 + * separate pathless layout routes. In this case, the route paths for + * both account/__public.tsx and account/__private.tsx is the same + * (/account), but we're again not expecting to match at that level. + * + * By only ignoring this check when the final portion of the filename is + * pathless, we will still detect path collisions such as: + * + * routes/parent/__pathless/foo.tsx + * routes/parent/__pathless2/foo.tsx + * + * and + * + * routes/parent/__pathless/index.tsx + * routes/parent/__pathless2/index.tsx + */ + if (uniqueRouteId && !isPathlessLayoutRoute) { if (uniqueRoutes.has(uniqueRouteId)) { throw new Error( - `Path ${JSON.stringify(fullPath)} defined by route ${JSON.stringify( + `Path ${JSON.stringify( + fullPath || "/" + )} defined by route ${JSON.stringify( routeId )} conflicts with route ${JSON.stringify( uniqueRoutes.get(uniqueRouteId)