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

multiple params, no parent #9

Closed
ryanflorence opened this issue May 13, 2022 · 4 comments
Closed

multiple params, no parent #9

ryanflorence opened this issue May 13, 2022 · 4 comments

Comments

@ryanflorence
Copy link

ryanflorence commented May 13, 2022

Awesome work! I love this.

I tried it on the revamp the react router docs site and had this route config:

app/routes
├── $lang.$ref
│   ├── $.tsx
│   └── index.tsx
├── $lang.$ref.tsx
├── healthcheck.tsx
└── index.tsx

Which creates this route tree with $ remix routes

<Routes>
  <Route file="root.tsx">
    <Route path="healthcheck" file="routes/healthcheck.tsx" />
    <Route path=":lang/:ref" file="routes/$lang.$ref.tsx">
      <Route index file="routes/$lang.$ref/index.tsx" />
      <Route path="*" file="routes/$lang.$ref/$.tsx" />
    </Route>
    <Route index file="routes/index.tsx" />
  </Route>
</Routes>

I ran the migration script and it kicked out:

app/flatroutes
├── $lang.$ref.$
├── $lang.$ref.index
├── $lang_.$ref
├── healthcheck
└── index

Which creates this incorrect route tree:

<Routes>
  <Route file="root.tsx">
    <Route path=":lang/:ref" file="flatroutes/$lang_.$ref/index.tsx" />
    <Route path="healthcheck" file="flatroutes/healthcheck/index.tsx" />
    <Route index file="flatroutes/index/index.tsx" />
  </Route>
</Routes>
  • Note the strange $lang_.$ref
  • Note the missing child routes under :lang/:ref

I'm realizing my examples never handled the case where a single route defines multiple segments <Route path=":lang/:ref" />, and it looks like the migration script and flatroutes aren't handling it either!

I changed my files to this, which I think is what the correct output of the migration script should be:

app/flatroutes
├── $lang.$ref
├── $lang.$ref.$
├── $lang.$ref.index
├── healthcheck
└── index

But the route config has an even bigger disappearing act with that 😶‍🌫️

<Routes>
  <Route file="root.tsx">
    <Route path="healthcheck" file="flatroutes/healthcheck/index.tsx" />
    <Route index file="flatroutes/index/index.tsx" />
  </Route>
</Routes>

It should look like this:

<Routes>
  <Route file="root.tsx">
    <Route path="healthcheck" file="routes/healthcheck.tsx" />
    <Route path=":lang/:ref" file="routes/$lang.$ref.tsx">
      <Route index file="routes/$lang.$ref/index.tsx" />
      <Route path="*" file="routes/$lang.$ref/$.tsx" />
    </Route>
    <Route index file="routes/index.tsx" />
  </Route>
</Routes>

The tricky bit here is when you see $lang.$ref and don't find a $lang to be the parent, we need to treat $lang.$ref as it's own route without needing a parent.

Of course, if the app developer adds a $lang then the route config will completely change on them, and I'd consider that expected behavior.

@ryanflorence
Copy link
Author

I also just tried to trick it with this, note all the $lang_

app/flatroutes
├── $lang_.$ref
├── $lang_.$ref.$
├── $lang_.$ref.index
├── healthcheck
└── index

And got another incorrect route config:

<Routes>
  <Route file="root.tsx">
    <Route path=":lang/:ref" file="flatroutes/$lang_.$ref/index.tsx" />
    <Route path=":lang/:ref/*" file="flatroutes/$lang_.$ref.$/index.tsx" />
    <Route path=":lang/:ref/" index file="flatroutes/$lang_.$ref.index/index.tsx" />
    <Route path="healthcheck" file="flatroutes/healthcheck/index.tsx" />
    <Route index file="flatroutes/index/index.tsx" />
  </Route>
</Routes>

Which brings up the question, should these two be equivalent?

app/flatroutes
├── $lang_.$ref
├── $lang_.$ref.$
├── $lang_.$ref.index
├── healthcheck
└── index
app/flatroutes
├── $lang.$ref
├── $lang.$ref.$
├── $lang.$ref.index
├── healthcheck
└── index

I think so. The explicit $lang_ would be the defensive way to do this so you can anticipate somebody adding a $lang route and not goofing it all up:

app/flatroutes
├── $lang <--- changes it all!
├── $lang.$ref
├── $lang.$ref.$
├── $lang.$ref.index
├── healthcheck
└── index

But in the absence of $lang I think those two file structures should generate the same route config:

<Routes>
  <Route file="root.tsx">
    <Route path="healthcheck" file="routes/healthcheck.tsx" />
    <Route path=":lang/:ref" file="routes/$lang.$ref.tsx">
      <Route index file="routes/$lang.$ref/index.tsx" />
      <Route path="*" file="routes/$lang.$ref/$.tsx" />
    </Route>
    <Route index file="routes/index.tsx" />
  </Route>
</Routes>

@kiliman
Copy link
Owner

kiliman commented May 13, 2022

Thanks for the issue and examples. I'll get this fixed.

@kiliman
Copy link
Owner

kiliman commented May 13, 2022

I think the trick is figuring out where to nest your layouts. Without folders to indicate parent, we're left with defaulting to nest under the previous segment. But if your parent was a flat layout like $lang.$ref, how to tell flat-routes to nest $.tsx under $lang.$ref instead of $ref.

Perhaps the trailing underscore should act like the / in default convention.

app/routes
├── nested
│   ├── $lang.$ref
│   │   ├── $.tsx
│   │   ├── index.tsx
│   │   ├── more
│   │   │   └── index.tsx
│   │   └── more.tsx
│   └── $lang.$ref.tsx
└── nested.tsx

Turns into

app/routes
├── nested
├── nested_.$lang.$ref
├── nested_.$lang.$ref_.$
├── nested_.$lang.$ref_.index
├── nested_.$lang.$ref_.more
└── nested_.$lang.$ref_.more.index

It assumes the parent segment is the parent layout, but if the parent segment ends with _, then collects all the segments up to the next _ (assumes all routes start with _ segment to denote root).

Again, flat-routes will assume that dots are slashes until it finds a segment ending in _, then it looks for the next _.

I think putting the _ suffix on the parent segment is more intuitive than putting on the child of the parent, and also solves the flat layout.

What do you think?

@kiliman
Copy link
Owner

kiliman commented May 27, 2022

I think I've fixed it!

app/ryanroutes2
├── $lang.$ref
│   └── _index.tsx
├── $lang.$ref.$
│   └── _index.tsx
├── $lang.$ref._index
│   └── _index.tsx
├── _index
│   └── _index.tsx
└── healthcheck
    └── _index.tsx

Which looks much nicer in IDE
image

<Routes>
  <Route file="root.tsx">
    <Route path=":lang/:ref" file="ryanroutes2/$lang.$ref/_index.tsx">
      <Route path="*" file="ryanroutes2/$lang.$ref.$/_index.tsx" />
      <Route index file="ryanroutes2/$lang.$ref._index/_index.tsx" />
    </Route>
    <Route index file="ryanroutes2/_index/_index.tsx" />
    <Route path="healthcheck" file="ryanroutes2/healthcheck/_index.tsx" />
  </Route>
</Routes>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants