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

Revert "Fix splat route index matching #6222

Merged
merged 4 commits into from
Apr 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/splat-index-matching-revert.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/dev": patch
---

Revert "Fix a bug in route matching that was preventing a single splat (`$.jsx`) route from matching a root `/` path"
2 changes: 1 addition & 1 deletion .changeset/splat-index-matching.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
"@remix-run/dev": patch
---

Fix a bug in route matching that wass preventing a single splat (`$.jsx`) route from matching a root `/` path
Fix a bug in route matching that was preventing a single splat (`$.jsx`) route from matching a root `/` path
2 changes: 1 addition & 1 deletion docs/tutorials/jokes.md
Original file line number Diff line number Diff line change
Expand Up @@ -6427,7 +6427,7 @@ function validatePassword(password: string) {
}

function validateUrl(url: string) {
let urls = ["/jokes", "/", "https://remix.run"];
const urls = ["/jokes", "/", "https://remix.run"];
if (urls.includes(url)) {
return url;
}
Expand Down
5 changes: 0 additions & 5 deletions integration/revalidate-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,6 @@ test.describe("Revalidation", () => {
}
`,

"app/routes/_index.jsx": js`
export default function Component() {
return <h1>Index</h1>;
}
`,
"app/routes/parent.jsx": js`
import { json } from "@remix-run/node";
import { Outlet, useLoaderData } from "@remix-run/react";
Expand Down
41 changes: 41 additions & 0 deletions integration/root-route-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { test, expect } from "@playwright/test";

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

test.describe("root route", () => {
let fixture: Fixture;
let appFixture: AppFixture;

test.beforeAll(async () => {
fixture = await createFixture({
files: {
"app/root.jsx": js`
export default function Root() {
return (
<html>
<body>
<h1>Hello Root!</h1>
</body>
</html>
);
}
`,
},
});

appFixture = await createAppFixture(fixture);
});

test.afterAll(() => {
appFixture.close();
});

test("matches the sole root route on /", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/");
await page.waitForSelector("h1");
expect(await app.getHtml("h1")).toMatch("Hello Root!");
});
});
106 changes: 2 additions & 104 deletions integration/splat-routes-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ import { test, expect } from "@playwright/test";
import { createFixture, js } from "./helpers/create-fixture";
import type { Fixture } from "./helpers/create-fixture";

let fixture: Fixture;

test.describe("rendering", () => {
let fixture: Fixture;

let ROOT_$ = "FLAT";
let ROOT_INDEX = "ROOT_INDEX";
let FLAT_$ = "FLAT";
Expand Down Expand Up @@ -128,105 +128,3 @@ test.describe("rendering", () => {
expect(await res.text()).toMatch(PARENTLESS_$);
});
});

test.describe("root splat route without index", () => {
test("matches routes correctly (v1)", async ({ page }) => {
fixture = await createFixture({
future: { v2_routeConvention: false },
files: {
"app/routes/$.jsx": js`
export default function Component() {
return <h1>Hello Splat</h1>
}
`,
},
});

let res = await fixture.requestDocument("/");
expect(await res.text()).toMatch("Hello Splat");

res = await fixture.requestDocument("/splat");
expect(await res.text()).toMatch("Hello Splat");

res = await fixture.requestDocument("/splat/deep/path");
expect(await res.text()).toMatch("Hello Splat");
});

test("matches routes correctly (v2)", async ({ page }) => {
fixture = await createFixture({
future: { v2_routeConvention: true },
files: {
"app/routes/$.jsx": js`
export default function Component() {
return <h1>Hello Splat</h1>
}
`,
},
});

let res = await fixture.requestDocument("/");
expect(await res.text()).toMatch("Hello Splat");

res = await fixture.requestDocument("/splat");
expect(await res.text()).toMatch("Hello Splat");

res = await fixture.requestDocument("/splat/deep/path");
expect(await res.text()).toMatch("Hello Splat");
});
});

test.describe("root splat route with index", () => {
test("matches routes correctly (v1)", async ({ page }) => {
fixture = await createFixture({
future: { v2_routeConvention: false },
files: {
"app/routes/index.jsx": js`
export default function Component() {
return <h1>Hello Index</h1>
}
`,
"app/routes/$.jsx": js`
export default function Component() {
return <h1>Hello Splat</h1>
}
`,
},
});

let res = await fixture.requestDocument("/");
expect(await res.text()).toMatch("Hello Index");

res = await fixture.requestDocument("/splat");
expect(await res.text()).toMatch("Hello Splat");

res = await fixture.requestDocument("/splat/deep/path");
expect(await res.text()).toMatch("Hello Splat");
});

test("matches routes correctly (v2)", async ({ page }) => {
fixture = await createFixture({
future: { v2_routeConvention: true },
files: {
"app/routes/_index.jsx": js`
export default function Component() {
return <h1>Hello Index</h1>
}
`,
"app/routes/$.jsx": js`
export default function Component() {
return <h1>Hello Splat</h1>
}
`,
},
});

let res = await fixture.requestDocument("/");
expect(await res.text()).toMatch("Hello Index");

res = await fixture.requestDocument("/splat");
expect(await res.text()).toMatch("Hello Splat");

res = await fixture.requestDocument("/splat/deep/path");
expect(await res.text()).toMatch("Hello Splat");
});
});
20 changes: 10 additions & 10 deletions packages/remix-dev/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,11 @@
Enable `unstable_dev` in `remix.config.js`:

```js
{
module.exports = {
future: {
"unstable_dev": true
}
}
unstable_dev: true,
},
};
```

## Remix App Server
Expand Down Expand Up @@ -182,12 +182,12 @@
import { devReady } from "@remix-run/node";

// Path to Remix's server build directory ('build/' by default)
let BUILD_DIR = path.join(process.cwd(), "build");
const BUILD_DIR = path.join(process.cwd(), "build");

// <code setting up your express server>

app.listen(3000, () => {
let build = require(BUILD_DIR);
const build = require(BUILD_DIR);
console.log("Ready: http://localhost:" + port);

// in development, call `devReady` _after_ your server is up and running
Expand All @@ -211,7 +211,7 @@
- You want to handle server updates yourself (e.g. via require cache purging)

```js
{
module.exports = {
future: {
unstable_dev: {
// Command to run your app server
Expand All @@ -227,9 +227,9 @@
// Whether the app server should be restarted when app is rebuilt
// See `Advanced > restart` for more
restart: false, // default: `true`
}
}
}
},
},
};
```

You can also configure via flags. For example:
Expand Down
1 change: 1 addition & 0 deletions packages/remix-dev/__tests__/readConfig-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ describe("readConfig", () => {
"root": Object {
"file": "root.tsx",
"id": "root",
"path": "",
},
},
"serverBuildPath": Any<String>,
Expand Down
2 changes: 1 addition & 1 deletion packages/remix-dev/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ export async function readConfig(
}

let routes: RouteManifest = {
root: { id: "root", file: rootRouteFile },
root: { path: "", id: "root", file: rootRouteFile },
};

let routesConvention: typeof flatRoutes;
Expand Down
20 changes: 10 additions & 10 deletions packages/remix-server-runtime/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,11 @@
Enable `unstable_dev` in `remix.config.js`:

```js
{
module.exports = {
future: {
"unstable_dev": true
}
}
unstable_dev: true,
},
};
```

## Remix App Server
Expand Down Expand Up @@ -170,12 +170,12 @@
import { devReady } from "@remix-run/node";

// Path to Remix's server build directory ('build/' by default)
let BUILD_DIR = path.join(process.cwd(), "build");
const BUILD_DIR = path.join(process.cwd(), "build");

// <code setting up your express server>

app.listen(3000, () => {
let build = require(BUILD_DIR);
const build = require(BUILD_DIR);
console.log("Ready: http://localhost:" + port);

// in development, call `devReady` _after_ your server is up and running
Expand All @@ -199,7 +199,7 @@
- You want to handle server updates yourself (e.g. via require cache purging)

```js
{
module.exports = {
future: {
unstable_dev: {
// Command to run your app server
Expand All @@ -215,9 +215,9 @@
// Whether the app server should be restarted when app is rebuilt
// See `Advanced > restart` for more
restart: false, // default: `true`
}
}
}
},
},
};
```

You can also configure via flags. For example:
Expand Down