Skip to content

Commit

Permalink
Merge branch 'dev' into dev
Browse files Browse the repository at this point in the history
  • Loading branch information
19Qingfeng committed May 25, 2023
2 parents 07af0e2 + d04a11e commit b0afbf7
Show file tree
Hide file tree
Showing 22 changed files with 1,122 additions and 33 deletions.
5 changes: 5 additions & 0 deletions .changeset/bubbled-response-cookies.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/server-runtime": patch
---

Automatically include set-cookie headers from bubbled thrown responses
9 changes: 9 additions & 0 deletions .changeset/stale-spoons-tie.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@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
5 changes: 5 additions & 0 deletions .changeset/v2-headers.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/server-runtime": minor
---

Added a new `future.v2_headers` future flag to opt into automatic inheriting of ancestor route `headers` functions so you do not need to export a `headers` function from every possible leaf route if you don't wish to.
1 change: 1 addition & 0 deletions docs/pages/api-development-strategy.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ Here's the current future flags in Remix v1 today:
| ------------------------ | --------------------------------------------------------------------- |
| `unstable_dev` | Enable the new development server (including HMR/HDR support) |
| `v2_errorBoundary` | Combine `ErrorBoundary`/`CatchBoundary` into a single `ErrorBoundary` |
| `v2_headers` | Leverage ancestor `headers` if children do not export `headers` |
| `v2_meta` | Enable the new API for your `meta` functions |
| `v2_normalizeFormMethod` | Normalize `useNavigation().formMethod` to be an uppercase HTTP Method |
| `v2_routeConvention` | Enable the flat routes style of file-based routing |
Expand Down
8 changes: 8 additions & 0 deletions docs/pages/v2.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,14 @@ routes

For more background on this change, see the [original "flat routes" proposal][flat-routes].

## Route `headers`

In Remix v2, the behavior for route `headers` functions is changing slightly. You can opt-into this new behavior ahead of time via the `future.v2_headers` flag in `remix.config.js`.

In v1, Remix would only use the result of the leaf "rendered" route `headers` function. It was your responsibility to add a `headers` function to every potential leaf and merge in `parentHeaders` accordingly. This can get tedious quickly and is also easy to forget to add a `headers` function when you add a new route, even if you want it to just share the same headers from it's parent.

In v2, Remix will use the deepest `headers` function that it finds in the rendered routes. This more easily allows you to share headers across routes from a common ancestor. Then as needed you can add `headers` functions to deeper routes if they require specific behavior.

## Route `meta`

In Remix v2, the signature for route `meta` functions and how Remix handles meta tags under the hood have changed.
Expand Down
33 changes: 31 additions & 2 deletions docs/route/headers.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export const headers: HeadersFunction = ({
actionHeaders,
loaderHeaders,
parentHeaders,
errorHeaders,
}) => ({
"X-Stretchy-Pants": "its for fun",
"Cache-Control": "max-age=300, s-maxage=3600",
Expand All @@ -33,7 +34,11 @@ export const headers: HeadersFunction = ({

Note: `actionHeaders` & `loaderHeaders` are an instance of the [Web Fetch API][headers] `Headers` class.

Because Remix has nested routes, there's a battle of the headers to be won when nested routes match. In this case, the deepest route wins. Consider these files in the routes directory:
If an action or a loader threw a `Response` and we're rendering a boundary, any headers from the thrown `Response` will be available in `errorHeaders`. This allows you to access headers from a child loader that threw in a parent error boundary.

## Nested Routes

Because Remix has nested routes, there's a battle of the headers to be won when nested routes match. The default behavior is that Remix only leverages the resulting headers from the leaf rendered route. Consider these files in the routes directory:

```
├── users.tsx
Expand All @@ -53,7 +58,11 @@ If we are looking at `/users/123/profile` then three routes are rendering:
</Users>
```

If all three define `headers`, the deepest module wins, in this case `profile.tsx`.
If all three define `headers`, the deepest module wins, in this case `profile.tsx`. However, if your `profile.tsx` loader threw and bubbled to a boundary in `userId.tsx` - then `userId.tsx`'s `headers` function would be used as it is the leaf rendered route.

<docs-info>
We realize that it can be tedious and error-prone to have to define `headers` on every possible leaf route so we're changing the current behavior in v2 behind the [`future.v2_headers`][v2_headers] flag.
</docs-info>

We don't want surprise headers in your responses, so it's your job to merge them if you'd like. Remix passes in the `parentHeaders` to your `headers` function. So `users.tsx` headers get passed to `$userId.tsx`, and then `$userId.tsx` headers are passed to `profile.tsx` headers.

Expand Down Expand Up @@ -118,5 +127,25 @@ export default function handleRequest(

Just keep in mind that doing this will apply to _all_ document requests, but does not apply to `data` requests (for client-side transitions for example). For those, use [`handleDataRequest`][handledatarequest].

## v2 Behavior

Since it can be tedious and error-prone to define a `header` function in every single possible leaf route, we're changing the behavior slightly in v2 and you can opt-into the new behavior via the `future.v2_headers` [Future Flag][future-flags] in `remix.config.js`.

When enabling this flag, Remix will now use the deepest `headers` function it finds in the renderable matches (up to and including the boundary route if an error is present). You'll still need to handle merging together headers as shown above for any `headers` functions above this route.

This means that, re-using the example above:

```
├── users.tsx
└── users
├── $userId.tsx
└── $userId
└── profile.tsx
```

If a user is looking at `/users/123/profile` and `profile.tsx` does not export a `headers` function, then Remix will use the return value of `$userId.tsx`'s `headers` function. If that file doesn't export one, then it will use the result of the one in `users.tsx`, and so on.

[headers]: https://developer.mozilla.org/en-US/docs/Web/API/Headers
[handledatarequest]: ../file-conventions/entry.server
[v2_headers]: #v2-behavior
[future-flags]: ../pages/api-development-strategy
132 changes: 132 additions & 0 deletions integration/conventional-routes-test.ts
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
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

0 comments on commit b0afbf7

Please sign in to comment.