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.

Sign up for GitHub

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

Jump to bottom

fix(remix-dev): adjust conventional routes logic for pathless layout routes #4421

Merged
merged 9 commits into from
May 24, 2023
Merged

fix(remix-dev): adjust conventional routes logic for pathless layout routes #4421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3000007
fix: avoid collisions on pathless routes with indices (#4355)
brophdawg11 Oct 25, 2022
8a64e5d
fix path collision detection fr pathless layout routes
brophdawg11 Oct 25, 2022
a41fd13
Update changeset
brophdawg11 Jan 24, 2023
63400c6
Add same fixes for flat routes
brophdawg11 Apr 21, 2023
a65d017
Merge branch 'dev' into brophdawg11/pathless-route-collision-3
brophdawg11 May 16, 2023
1879cad
Add more tests for folder/route.tsx flat routes convention
brophdawg11 May 16, 2023
93c48d0
Update integration tests for v2 routes
brophdawg11 May 22, 2023
cf00df9
Move route parentId assignment before the short circuit
brophdawg11 May 22, 2023
fc33f83
Merge branch 'dev' into brophdawg11/pathless-route-collision-3
brophdawg11 May 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/stale-spoons-tie.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@remix-run/dev": patch
---

- Fix route ranking bug with pathless layout route next to a sibling index route
- Under the hood this is done by removing the trailing slash from all generated `path` values since the number of slash-delimited segments counts towards route ranking so the trailing slash incorrectly increases the score for routes

- Support sibling pathless layout routes by removing pathless layout routes from the unique route path checks in conventional route generation since they inherently trigger duplicate paths
132 changes: 132 additions & 0 deletions integration/conventional-routes-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
import { test, expect } from "@playwright/test";

import { PlaywrightFixture } from "./helpers/playwright-fixture";
import type { Fixture, AppFixture } from "./helpers/create-fixture";
import { createAppFixture, createFixture, js } from "./helpers/create-fixture";

let fixture: Fixture;
let appFixture: AppFixture;

test.beforeAll(async () => {
fixture = await createFixture({
files: {
"app/root.tsx": js`
import { Link, Outlet, Scripts, useMatches } from "@remix-run/react";

export default function App() {
let matches = 'Number of matches: ' + useMatches().length;
return (
<html lang="en">
<body>
<nav>
<Link to="/nested">/nested</Link>
<br />
<Link to="/nested/foo">/nested/foo</Link>
<br />
</nav>
<p>{matches}</p>
<Outlet />
<Scripts />
</body>
</html>
);
}
`,
"app/routes/nested/index.jsx": js`
export default function Index() {
return <h1>Index</h1>;
}
`,
"app/routes/nested/__pathless.jsx": js`
import { Outlet } from "@remix-run/react";

export default function Layout() {
return (
<>
<div>Pathless Layout</div>
<Outlet />
</>
);
}
`,
"app/routes/nested/__pathless/foo.jsx": js`
export default function Foo() {
return <h1>Foo</h1>;
}
`,
"app/routes/nested/__pathless2.jsx": js`
import { Outlet } from "@remix-run/react";

export default function Layout() {
return (
<>
<div>Pathless 2 Layout</div>
<Outlet />
</>
);
}
`,
"app/routes/nested/__pathless2/bar.jsx": js`
export default function Bar() {
return <h1>Bar</h1>;
}
`,
},
});

appFixture = await createAppFixture(fixture);
});

test.afterAll(async () => appFixture.close());

test.describe("with JavaScript", () => {
runTests();
});

test.describe("without JavaScript", () => {
test.use({ javaScriptEnabled: false });
runTests();
});

/**
* Routes for this test look like this, for reference for the matches assertions:
*
* <Routes>
* <Route file="root.jsx">
* <Route path="nested" file="routes/nested/__pathless.jsx">
* <Route path="foo" file="routes/nested/__pathless/foo.jsx" />
* </Route>
* <Route path="nested" index file="routes/nested/index.jsx" />
* <Route index file="routes/index.jsx" />
* </Route>
* </Routes>
*/

function runTests() {
test("displays index page and not pathless layout page", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/nested");
expect(await app.getHtml()).toMatch("Index");
expect(await app.getHtml()).not.toMatch("Pathless Layout");
expect(await app.getHtml()).toMatch("Number of matches: 2");
});

test("displays page inside of pathless layout", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/nested/foo");
expect(await app.getHtml()).not.toMatch("Index");
expect(await app.getHtml()).toMatch("Pathless Layout");
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");
});
}
129 changes: 129 additions & 0 deletions integration/flat-routes-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,3 +312,132 @@ test.describe("", () => {
expect(buildOutput).not.toContain(`Route Path Collision`);
});
});

test.describe("pathless routes and route collisions", () => {
test.beforeAll(async () => {
fixture = await createFixture({
future: { v2_routeConvention: true },
files: {
"app/root.tsx": js`
import { Link, Outlet, Scripts, useMatches } from "@remix-run/react";

export default function App() {
let matches = 'Number of matches: ' + useMatches().length;
return (
<html lang="en">
<body>
<nav>
<Link to="/nested">/nested</Link>
<br />
<Link to="/nested/foo">/nested/foo</Link>
<br />
</nav>
<p>{matches}</p>
<Outlet />
<Scripts />
</body>
</html>
);
}
`,
"app/routes/nested._index.jsx": js`
export default function Index() {
return <h1>Index</h1>;
}
`,
"app/routes/nested._pathless.jsx": js`
import { Outlet } from "@remix-run/react";

export default function Layout() {
return (
<>
<div>Pathless Layout</div>
<Outlet />
</>
);
}
`,
"app/routes/nested._pathless.foo.jsx": js`
export default function Foo() {
return <h1>Foo</h1>;
}
`,
"app/routes/nested._pathless2.jsx": js`
import { Outlet } from "@remix-run/react";

export default function Layout() {
return (
<>
<div>Pathless 2 Layout</div>
<Outlet />
</>
);
}
`,
"app/routes/nested._pathless2.bar.jsx": js`
export default function Bar() {
return <h1>Bar</h1>;
}
`,
},
});

appFixture = await createAppFixture(fixture);
});

test.afterAll(async () => appFixture.close());

test.describe("with JavaScript", () => {
runTests();
});

test.describe("without JavaScript", () => {
test.use({ javaScriptEnabled: false });
runTests();
});

/**
* Routes for this test look like this, for reference for the matches assertions:
*
* <Routes>
* <Route file="root.jsx">
* <Route path="nested" file="routes/nested/__pathless.jsx">
* <Route path="foo" file="routes/nested/__pathless/foo.jsx" />
* </Route>
* <Route path="nested" index file="routes/nested/index.jsx" />
* <Route index file="routes/index.jsx" />
* </Route>
* </Routes>
*/

function runTests() {
test("displays index page and not pathless layout page", async ({
page,
}) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/nested");
expect(await app.getHtml()).toMatch("Index");
expect(await app.getHtml()).not.toMatch("Pathless Layout");
expect(await app.getHtml()).toMatch("Number of matches: 2");
});

test("displays page inside of pathless layout", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/nested/foo");
expect(await app.getHtml()).not.toMatch("Index");
expect(await app.getHtml()).toMatch("Pathless Layout");
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");
});
}
});
Loading
Loading