Repository: remix-run/remix
Pull request #5560 (2023-02-24 19:58:13)

chore(remix-dev): remove broken param collision check
hit a snag in a demo app as it complained there was a conflict when there wasn't.

```log
⚠️ Route Path Collision: "/:username"

The following routes all define the same URL, only the first one will be used

🟢 routes/_user.$username.tsx
⭕️️ routes/sneakers.$sneakerId.tsx
```

<!--

👋 Hey, thanks for your interest in contributing to Remix!

If this is a simple docs change, go ahead and delete all this.

**Please ask first before starting work on any significant new features.**

It's never a fun experience to have your pull request declined after investing a
lot of time and effort into a new feature. To avoid this from happening, we
request that contributors create a
[Feature Request discussion](https://github.com/remix-run/remix/discussions/new?category=ideas)
to first discuss any significant new features.

https://github.com/remix-run/remix/blob/main/CONTRIBUTING.md

Please fill in or delete each item below:

-->

Closes: #

- [ ] Docs
- [ ] Tests

Testing Strategy:

<!--
Please explain how you tested this. For example:

> This test covers this code: <link to test>

Or

> I opened up my windows machine and ran this script:
>
> ```
> npx create-remix@0.0.0-experimental-7e420ee3 --template remix my-test
> cd my-test
> npm run dev
> ```
-->


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

Latest commit: 0ad2a2a4bbc18da2c9d403731ca6cad5304cffa6

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/logan/remove-broken-param-collision?filename=.changeset/khaki-parents-whisper.md&value=---%0A%22remix%22%3A%20patch%0A%22%40remix-run%2Fdev%22%3A%20patch%0A---%0A%0Achore(remix-dev)%3A%20remove%20broken%20param%20collision%20check%0A)


mcansh: i think we can skip the changeset here as this was never in a stable release yet
brophdawg11: Ah - @mcansh we will need a changeset here since this points to `release-next` - otherwise it won't trigger a new release.  But we can remove the actual line from the final release notes since this never shipped
mcansh: > Ah - @mcansh we will need a changeset here since this points to `release-next` - otherwise it won't trigger a new release. But we can remove the actual line from the final release notes since this never shipped

👍 this is the fella to remove `.changeset/afraid-tigers-occur.md`

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/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/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",