Repository: remix-run/remix
Pull request #5590 (2023-02-27 19:48:31)

Move scroll test to it's own file
Reverts an unintentional change to `bug-report-test.ts` (#3171) and moves the passing test to it's own `scroll-test.ts` file


changeset-bot[bot]: ###  ⚠️  No Changeset found

Latest commit: a15027e41f449d752ba8a105b4f653826c47a6fe

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. **If these changes should result in a version bump, you need to add a changeset.**

<details><summary>This PR includes no changesets</summary>

  When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

</details>

[Click here to learn what changesets are, and how to add one](https://github.com/changesets/changesets/blob/master/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add a changeset to this PR](https://github.com/remix-run/remix/new/brophdawg11/revert-bug-test?filename=.changeset/kind-drinks-occur.md&value=---%0A%0A---%0A%0AMove%20scroll%20test%20to%20it's%20own%20file%0A)



Patch:
diff --git a/.changeset/afraid-tigers-occur.md b/.changeset/afraid-tigers-occur.md
new file mode 100644
index 000000000..48be7961e
--- /dev/null
+++ b/.changeset/afraid-tigers-occur.md
@@ -0,0 +1,6 @@
+---
+"remix": patch
+"@remix-run/dev": patch
+---
+
+chore(remix-dev): remove broken param collision check
diff --git a/.changeset/red-kids-look.md b/.changeset/red-kids-look.md
new file mode 100644
index 000000000..e2ad1ee43
--- /dev/null
+++ b/.changeset/red-kids-look.md
@@ -0,0 +1,5 @@
+---
+"remix": patch
+---
+
+Restore bug-report-test.ts (Can be removed from final release notes)
diff --git a/packages/remix-dev/config/flat-routes.ts b/packages/remix-dev/config/flat-routes.ts
index eb9224c23..2d5fc19d2 100644
--- a/packages/remix-dev/config/flat-routes.ts
+++ b/packages/remix-dev/config/flat-routes.ts
@@ -360,45 +360,9 @@ function getRouteMap(
     return b.segments.length - a.segments.length;
   });
 
-  for (let i = 0; i < routes.length; i++) {
-    let routeInfo = routes[i];
+  for (let routeInfo of routes) {
     // update parentIds for all routes
     routeInfo.parentId = findParentRouteId(routeInfo, nameMap);
-
-    // remove routes that conflict with other routes
-    let nextRouteInfo = routes[i + 1];
-    if (!nextRouteInfo) continue;
-
-    let segments = routeInfo.segments;
-    let nextSegments = nextRouteInfo.segments;
-    // if segment count is different, there can't be a conflict
-    if (segments.length !== nextSegments.length) continue;
-
-    for (let k = 0; k < segments.length; k++) {
-      let segment = segments[k];
-      let nextSegment = nextSegments[k];
-
-      // if segments are different, but they're both dynamic, there's a conflict
-      if (
-        segment !== nextSegment &&
-        segment.startsWith(":") &&
-        nextSegment.startsWith(":")
-      ) {
-        let currentConflicts = conflicts.get(routeInfo.path || "/");
-        // collect conflicts for later reporting, marking the next route as the conflicting route
-        if (!currentConflicts) {
-          conflicts.set(routeInfo.path || "/", [routeInfo, nextRouteInfo]);
-        } else {
-          currentConflicts.push(nextRouteInfo);
-          conflicts.set(routeInfo.path || "/", currentConflicts);
-        }
-
-        // remove conflicting route
-        routeMap.delete(nextRouteInfo.id);
-
-        continue;
-      }
-    }
   }
 
   // report conflicts


Test patch:
diff --git a/integration/bug-report-test.ts b/integration/bug-report-test.ts
index f4f7c27f3..c8a9f18ac 100644
--- a/integration/bug-report-test.ts
+++ b/integration/bug-report-test.ts
@@ -48,37 +48,27 @@ test.beforeAll(async () => {
     ////////////////////////////////////////////////////////////////////////////
     files: {
       "app/routes/index.jsx": js`
-        import { redirect } from "@remix-run/node";
-        import { Form } from "@remix-run/react";
-        
-        export const action = async () => {
-          await new Promise((resolve) => setTimeout(resolve, 500));
-        
-          return redirect("/test");
-        };
-        
+        import { json } from "@remix-run/node";
+        import { useLoaderData, Link } from "@remix-run/react";
+
+        export function loader() {
+          return json("pizza");
+        }
+
         export default function Index() {
+          let data = useLoaderData();
           return (
             <div>
-              <h1>Scroll down to test</h1>
-        
-              <Form method="post" action="/?index" style={{ marginTop: "150vh" }}>
-                <h1>Test here</h1>
-                <button type="submit">test</button>
-              </Form>
+              {data}
+              <Link to="/burgers">Other Route</Link>
             </div>
-          );
-        }      
+          )
+        }
       `,
 
-      "app/routes/test.jsx": js`
+      "app/routes/burgers.jsx": js`
         export default function Index() {
-          return (
-            <div>
-              <h1>This is the top</h1>
-              <h1 style={{ marginTop: "150vh" }}>This is the bottom</h1>
-            </div>
-          )
+          return <div>cheeseburger</div>;
         }
       `,
     },
@@ -97,22 +87,16 @@ test.afterAll(() => {
 // add a good description for what you expect Remix to do 👇🏽
 ////////////////////////////////////////////////////////////////////////////////
 
-test("page scroll should be at the top on the new page", async ({ page }) => {
+test("[description of what you expect it to do]", async ({ page }) => {
   let app = new PlaywrightFixture(appFixture, page);
   // You can test any request your app might get using `fixture`.
   let response = await fixture.requestDocument("/");
-  expect(await response.text()).toMatch("Scroll down to test");
+  expect(await response.text()).toMatch("pizza");
 
   // If you need to test interactivity use the `app`
   await app.goto("/");
-  // scroll to the bottom
-  await page.evaluate(() => window.scrollTo(0, document.body.scrollHeight));
-  await app.clickSubmitButton("/?index");
-
-  expect(await app.getHtml()).toMatch("This is the bottom");
-
-  let newScrollY = await page.evaluate(() => window.scrollY);
-  expect(newScrollY).toBe(0);
+  await app.clickLink("/burgers");
+  expect(await app.getHtml()).toMatch("cheeseburger");
 
   // If you're not sure what's going on, you can "poke" the app, it'll
   // automatically open up in your browser for 20 seconds, so be quick!
diff --git a/integration/scroll-test.ts b/integration/scroll-test.ts
new file mode 100644
index 000000000..0ab416e7a
--- /dev/null
+++ b/integration/scroll-test.ts
@@ -0,0 +1,68 @@
+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/routes/index.jsx": js`
+        import { redirect } from "@remix-run/node";
+        import { Form } from "@remix-run/react";
+
+        export function action() {
+          return redirect("/test");
+        };
+
+        export default function Component() {
+          return (
+            <>
+              <h1>Index Page - Scroll Down</h1>
+              <Form method="post" style={{ marginTop: "150vh" }}>
+                <button type="submit">Submit</button>
+              </Form>
+            </>
+          );
+        }
+      `,
+
+      "app/routes/test.jsx": js`
+        export default function Component() {
+          return (
+            <>
+              <h1 id="redirected">Redirected!</h1>
+              <p style={{ marginTop: "150vh" }}>I should not be visible!!</p>
+            </>
+          );
+        }
+      `,
+    },
+  });
+
+  // This creates an interactive app using puppeteer.
+  appFixture = await createAppFixture(fixture);
+});
+
+test.afterAll(() => {
+  appFixture.close();
+});
+
+test("page scroll should be at the top on the new page", async ({ page }) => {
+  let app = new PlaywrightFixture(appFixture, page);
+  await app.goto("/");
+
+  // Scroll to the bottom and submit
+  await page.evaluate(() => window.scrollTo(0, document.body.scrollHeight));
+  let scroll = await page.evaluate(() => window.scrollY);
+  expect(scroll).toBeGreaterThan(0);
+  await app.clickSubmitButton("/?index");
+  await page.waitForSelector("#redirected");
+
+  // Ensure we scrolled back to the top
+  scroll = await page.evaluate(() => window.scrollY);
+  expect(scroll).toBe(0);
+});
diff --git a/packages/remix-dev/__tests__/flat-routes-test.ts b/packages/remix-dev/__tests__/flat-routes-test.ts
index 437d1e321..6d947295b 100644
--- a/packages/remix-dev/__tests__/flat-routes-test.ts
+++ b/packages/remix-dev/__tests__/flat-routes-test.ts
@@ -619,6 +619,31 @@ describe("flatRoutes", () => {
     }
   });
 
+  describe("doesn't warn when there's not a route collision", () => {
+    let consoleError = jest
+      .spyOn(global.console, "error")
+      .mockImplementation(() => {});
+
+    afterEach(consoleError.mockReset);
+
+    test("same number of segments and the same dynamic segment index", () => {
+      let testFiles = [
+        "routes/_user.$username.tsx",
+        "routes/sneakers.$sneakerId.tsx",
+      ];
+
+      let routeManifest = flatRoutesUniversal(
+        APP_DIR,
+        testFiles.map((file) => path.join(APP_DIR, normalizePath(file)))
+      );
+
+      let routes = Object.values(routeManifest);
+
+      expect(routes).toHaveLength(testFiles.length);
+      expect(consoleError).not.toHaveBeenCalled();
+    });
+  });
+
   describe("warns when there's a route collision", () => {
     let consoleError = jest
       .spyOn(global.console, "error")
@@ -627,7 +652,6 @@ describe("flatRoutes", () => {
     afterEach(consoleError.mockReset);
 
     test("index files", () => {
-      // we'll add file manually before running the tests
       let testFiles = [
         "routes/_dashboard._index.tsx",
         "routes/_landing._index.tsx",
@@ -666,7 +690,7 @@ describe("flatRoutes", () => {
       );
     });
 
-    test("same path, different param name", () => {
+    test.skip("same path, different param name", () => {
       // we'll add file manually before running the tests
       let testFiles = [
         "routes/products.$pid.tsx",