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

brophdawg11 · 2022-10-25T22:07:14Z

tl;dr;

This fixes 2 issues ion our conventional routes logic:

Removes the trailing slash from all generated path values since # of slash-delimited segments counts towards route ranking so the trailing slash incorrectly increases the scoring for routes.
Removes pathless layout routes from the unique route path checks since they inherently trigger duplicate paths. This allows us to support sibling pathless layouts.

Deets

#4355 had an integration test failure once merged to dev due to conflicts with an unrelated PR that merged an integration tests with an index file inside a pathless layout route folder. This uncovered a bit of a rabbit hole into some details around our path collision detection in conventionalRoutes.ts.

Before #4355, index routes inside pathless layout routes were fine because they had a trailing slash hanging around on the "path", so our uniqueRouteIds structure looked like:

{
  'parent': 'routes/parent.tsx',
  'parent/': 'routes/parent/__pathless.tsx',
  'parent?index': 'routes/parent/__pathless/index.tsx'
}

Once we got rid of the trailing slash in #4355 (to fix #3014) we now start failing the build on the above since the "path" (the key in the Map) is identical for both matches.

This turns out to also be the reason that sibling pathless layout routes fail - even though the following is valid in React Router:

<Routes>
  <Route file="root.tsx">
    <Route path="parent" file="routes/parent.jsx">
      <Route file="routes/parent/__a.jsx">
        <Route path="a-child" file="routes/parent/__a/a-child.jsx" />
      </Route>
      <Route file="routes/parent/__b.jsx">
        <Route path="b-child" file="routes/parent/__b/b-child.jsx" />
      </Route>
    </Route>
  </Route>
</Routes>

With the current logic, we end up reporting a path collision on /parent between routes/parent/__a.jsx and routes/parent/__b.jsx even though they have mutually exclusive children routes.

What I ended up doing in here is just ignoring path collision detection only for pathless-layout route files, since by definition they sort of introduce potential path collisions that the user should be aware of and accounting for (we even warn them in the docs!). We still check for collisions on any children route files inside a __pathless/ directory - which lets us detect if they include non-mutually-exclusive route paths beneath a pathless layout route folder.

So at the end of the day we get 2 things from this PR:

How to use nested route less layout the right way #3014 is fixed and we correctly route to index routes next to pathless layout routes ✅
We also get some new functionality and can support sibling pathless layout routes 😄

changeset-bot · 2022-10-25T22:07:17Z

🦋 Changeset detected

Latest commit: fc33f83

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 18 packages

Name	Type
@remix-run/dev	Patch
create-remix	Patch
@remix-run/css-bundle	Patch
remix	Patch
@remix-run/architect	Patch
@remix-run/cloudflare	Patch
@remix-run/cloudflare-pages	Patch
@remix-run/cloudflare-workers	Patch
@remix-run/deno	Patch
@remix-run/eslint-config	Patch
@remix-run/express	Patch
@remix-run/netlify	Patch
@remix-run/node	Patch
@remix-run/react	Patch
@remix-run/serve	Patch
@remix-run/server-runtime	Patch
@remix-run/testing	Patch
@remix-run/vercel	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

brophdawg11 · 2022-10-26T17:55:33Z

packages/remix-dev/config/routesConvention.ts

@@ -188,6 +240,8 @@ export function createRoutePath(partialRouteId: string): string | undefined {

  if (rawSegmentBuffer === "index" && result.endsWith("index")) {
    result = result.replace(/\/?index$/, "");
+  } else {
+    result = result.replace(/\/$/, "");


Align route paths so we never include trailing slashes on <Route path> entries, which messes with ranking since /-delimited segments count in ranking

Fixes #3014

brophdawg11 · 2023-01-18T16:50:27Z

packages/remix-dev/config/routesConvention.ts

+       *   routes/parent/__pathless/index.tsx
+       *   routes/parent/__pathless2/index.tsx
+       */
+      if (uniqueRouteId && !isPathlessLayoutRoute) {


Fixes #3327

chaance

This looks awesome. Just a minor request for the changeset. Also, should we add similar tests for flat route conventions?

.changeset/stale-spoons-tie.md

brophdawg11 · 2023-01-24T20:33:15Z

yeah I can look into the same tests/logic for flat routes 👍

sixers · 2023-03-17T07:52:11Z

Hey, any update on this? I have to patch-package and apply this hotfix each time I upgrade Remix. It would be awesome to finally merge this fix.

brophdawg11 · 2023-03-17T14:45:10Z

@sixers This is on hold since I'm unsure if it's needed when using new route convention. The code this PR is fixing will be removed in v2 but made available through another package so you can continue using the nested folder structure via the remix.config routes method. If your team happens to try out flat routes I'd love to know if it resolves this issue. Otherwise I plan to dig back into this once we stabilize some of the ongoing work for 1.15.0.

sixers · 2023-03-17T15:26:14Z

@brophdawg11 Yes, it's needed, the issue also exists in V2 route convention.

chaance · 2023-04-08T14:50:00Z

@brophdawg11 Just so it's on your radar, you probably want to keep the v1 compat package in sync with these kind of fixes. https://github.com/remix-run/v1-compat-utils

* fix: avoid collisions on pathless routes with indices * Add changeset * Remove unused import from e2e test

Chance is no longer part of the team 😢

brophdawg11 · 2023-05-16T18:21:16Z

Added PR for @remix-run/v1-route-convention in remix-run/v1-compat-utils#20

github-actions · 2023-05-25T07:06:52Z

🤖 Hello there,

We just published version v0.0.0-nightly-33cc4c0-20230525 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

github-actions · 2023-06-02T18:32:00Z

🤖 Hello there,

We just published version 1.17.0-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

github-actions · 2023-06-06T19:30:54Z

🤖 Hello there,

We just published version 1.17.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

remix-cla-bot bot added the CLA Signed label Oct 25, 2022

brophdawg11 commented Oct 26, 2022

View reviewed changes

MichaelDeBoey changed the title ~~adjust conventional routes logic for pathless layout routes~~ fix(remix-dev): adjust conventional routes logic for pathless layout routes Oct 26, 2022

MichaelDeBoey added the package:dev label Oct 26, 2022

brophdawg11 mentioned this pull request Nov 22, 2022

fix(remix-dev): support multiple nested pathless layout routes #3331

Closed

2 tasks

brophdawg11 commented Jan 18, 2023

View reviewed changes

brophdawg11 assigned chaance Jan 24, 2023

machour mentioned this pull request Jan 24, 2023

[Bug]: Layout route with more than one pathless layout causes invalid conflict #3327

Closed

machour added the feat:routing label Jan 24, 2023

chaance previously requested changes Jan 24, 2023

View reviewed changes

.changeset/stale-spoons-tie.md Outdated Show resolved Hide resolved

brophdawg11 linked an issue Jan 24, 2023 that may be closed by this pull request

[Bug]: Layout route with more than one pathless layout causes invalid conflict #3327

Closed

brophdawg11 assigned brophdawg11 and unassigned chaance Jan 24, 2023

brophdawg11 added 3 commits April 21, 2023 15:42

fix: avoid collisions on pathless routes with indices (#4355)

3000007

* fix: avoid collisions on pathless routes with indices * Add changeset * Remove unused import from e2e test

fix path collision detection fr pathless layout routes

8a64e5d

Update changeset

a41fd13

brophdawg11 force-pushed the brophdawg11/pathless-route-collision-3 branch from 182cd3a to a41fd13 Compare April 21, 2023 19:42

Add same fixes for flat routes

63400c6

This was referenced Apr 26, 2023

chore: Update version for release (pre) #6193

Merged

chore: Update version for release #6236

Closed

chore: Update version for release #6264

Merged

MichaelDeBoey force-pushed the brophdawg11/pathless-route-collision-3 branch from 63400c6 to 5f07ba7 Compare May 1, 2023 23:40

MichaelDeBoey requested a review from mcansh May 1, 2023 23:41

MichaelDeBoey requested a review from jacob-ebey May 1, 2023 23:41

brophdawg11 force-pushed the brophdawg11/pathless-route-collision-3 branch from 5f07ba7 to 63400c6 Compare May 16, 2023 18:03

Merge branch 'dev' into brophdawg11/pathless-route-collision-3

a65d017

brophdawg11 mentioned this pull request May 16, 2023

fix(v1-route-convention): allow sibling pathless layout routes remix-run/v1-compat-utils#20

Merged

Add more tests for folder/route.tsx flat routes convention

1879cad

brophdawg11 added 3 commits May 22, 2023 12:03

Update integration tests for v2 routes

93c48d0

Move route parentId assignment before the short circuit

cf00df9

Merge branch 'dev' into brophdawg11/pathless-route-collision-3

fc33f83

jacob-ebey approved these changes May 23, 2023

View reviewed changes

brophdawg11 merged commit 5f808a4 into dev May 24, 2023

brophdawg11 deleted the brophdawg11/pathless-route-collision-3 branch May 24, 2023 18:03

github-actions bot mentioned this pull request May 24, 2023

chore: Update version for release (pre) remix-run/v1-compat-utils#21

Merged

This was referenced Jun 2, 2023

chore: Update version for release (pre) #6525

Closed

chore: Update version for release (pre) #6526

Merged

github-actions bot mentioned this pull request Jun 6, 2023

chore: Update version for release #6550

Merged

brophdawg11 mentioned this pull request Jun 20, 2023

Fix bug with pathless layout routes beneath nested path segments #6649

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

brophdawg11 commented Oct 25, 2022 •

edited

Loading

changeset-bot bot commented Oct 25, 2022 •

edited

Loading

brophdawg11 Oct 26, 2022

brophdawg11 Jan 18, 2023

brophdawg11 Jan 18, 2023

chaance left a comment

brophdawg11 commented Jan 24, 2023

sixers commented Mar 17, 2023

brophdawg11 commented Mar 17, 2023

sixers commented Mar 17, 2023

chaance commented Apr 8, 2023

brophdawg11 commented May 16, 2023

github-actions bot commented May 25, 2023

github-actions bot commented Jun 2, 2023

github-actions bot commented Jun 6, 2023

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

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

Conversation

brophdawg11 commented Oct 25, 2022 • edited Loading

tl;dr;

Deets

changeset-bot bot commented Oct 25, 2022 • edited Loading

🦋 Changeset detected

brophdawg11 Oct 26, 2022

Choose a reason for hiding this comment

brophdawg11 Jan 18, 2023

Choose a reason for hiding this comment

brophdawg11 Jan 18, 2023

Choose a reason for hiding this comment

chaance left a comment

Choose a reason for hiding this comment

brophdawg11 commented Jan 24, 2023

sixers commented Mar 17, 2023

brophdawg11 commented Mar 17, 2023

sixers commented Mar 17, 2023

chaance commented Apr 8, 2023

brophdawg11 commented May 16, 2023

github-actions bot commented May 25, 2023

github-actions bot commented Jun 2, 2023

github-actions bot commented Jun 6, 2023

brophdawg11 commented Oct 25, 2022 •

edited

Loading

changeset-bot bot commented Oct 25, 2022 •

edited

Loading