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

Trailing underscore at params affect useParams #11

Closed
alexluong opened this issue May 30, 2022 · 14 comments
Closed

Trailing underscore at params affect useParams #11

alexluong opened this issue May 30, 2022 · 14 comments

Comments

@alexluong
Copy link

Hi, I'm trying out remix-flat-routes and when I use routes with trailing underscore at params, it affects the useParams as well as params in loader / action.

I have to resort to creating a getParam abstraction but just wondering if there's a good way to solve this within the library itself.


Just to clarify in case my description is not clear enough, for example, I have these routes:

/app/$organizationSlug.tsx
/app/$organizationSlug_/other.tsx` --> useParams() will return { organizationSlug_: "slug" }
@alexluong alexluong changed the title Params name with trailing underscore Trailing underscore at params affect useParams May 30, 2022
@kiliman
Copy link
Owner

kiliman commented May 30, 2022

You’re right. The underscore should be stripped off the route path. Thanks for the report.

BTW I’ve changed how it computes parent routes. Is there a reason you need the trailing underscore?

Can you post your routes? Thanks! 🙏

@alexluong
Copy link
Author

For example, let's say I have Organization and Project entities where projects belong to organizations.

I have these routes (URL routes, not Remix):

Organization routes

/app/$organizationSlug
/app/$organizationSlug/edit

Project routes

/app/$organizationSlug/projects/new
/app/$organizationSlug/projects/$projectId
/app/$organizationSlug/projects/$projectId/edit

I want organization routes & project routes to be wrapped in different layouts. Here are the Remix routes I have.

/app/$organizationSlug
/app/$organizationSlug/edit

/app/$organizationSlug_/projects/new
/app/$organizationSlug_/projects/$projectId
/app/$organizationSlug_/projects/$projectId/edit

Let me know if there's a better way to go about this.

I'm on v0.4.0 btw, so the latest version I think.

@kiliman
Copy link
Owner

kiliman commented May 30, 2022

Thanks for the examples. They’ll help me fix the problem.

@kiliman
Copy link
Owner

kiliman commented May 31, 2022

Ok, I'm working on the test cases. What are the parent layout for each file?

Right now, without the underscore, it looks like this:

<Routes>
  <Route file="root.tsx">
    <Route path="app/:organizationSlug" file="routes/app.$organizationSlug.tsx">
      <Route path="edit" file="routes/app.$organizationSlug.edit.tsx" />
      <Route path="projects/:projectId" file="routes/app.$organizationSlug.projects.$projectId.tsx">
        <Route path="edit" file="routes/app.$organizationSlug.projects.$projectId.edit.tsx" />
      </Route>
      <Route path="projects/new" file="routes/app.$organizationSlug.projects.new.tsx" />
    </Route>
  </Route>
</Routes>

As you can see, everything is slotted under app/:organizationSlug.

I'm trying to figure out where you expect the project routes to be defined under?

@alexluong
Copy link
Author

This is perfect. I'm still sorting out the parent layout but this is basically the expected behavior for this issue.

I don't have the projects parent layout currently but I imagine I may want to have something like this

/app/$organizationSlug_/_projects
/app/$organizationSlug_/_projects/projects/new
/app/$organizationSlug_/_projects/projects/$projectId
/app/$organizationSlug_/_projects/projects/$projectId/edit

The expected routing for that would be something like so

<Routes>
  <Route file="root.tsx">
    <Route path="app/:organizationSlug" file="routes/app.$organizationSlug.tsx">
      <Route path="edit" file="routes/app.$organizationSlug.edit.tsx" />
      <Route file="routes/app/$organizationSlug_/_projects.tsx">
        <Route path="projects/:projectId" file="routes/app.$organizationSlug.projects.$projectId.tsx">
          <Route path="edit" file="routes/app.$organizationSlug.projects.$projectId.edit.tsx" />
        </Route>
        <Route path="projects/new" file="routes/app.$organizationSlug.projects.new.tsx" />
      </Route>
    </Route>
  </Route>
</Routes>

@kiliman
Copy link
Owner

kiliman commented May 31, 2022

Ok, I don't think you need the trailing _, nor do you need the pathless layout.

const routeList = [
  'app.$organizationSlug.tsx',
  'app.$organizationSlug.edit.tsx',
  'app.$organizationSlug.projects.tsx',  // layout for all projects routes
  'app.$organizationSlug.projects.$projectId.tsx',
  'app.$organizationSlug.projects.$projectId.edit.tsx',
  'app.$organizationSlug.projects.new.tsx',
]

<Routes>
  <Route file="root.tsx">
    <Route path="app/:organizationSlug" file="routes/app.$organizationSlug.tsx">
      <Route path="edit" file="routes/app.$organizationSlug.edit.tsx">
      <Route path="projects" file="routes/app.$organizationSlug.projects.tsx">
        <Route path=":projectId" file="routes/app.$organizationSlug.projects.$projectId.tsx">
          <Route path="edit" file="routes/app.$organizationSlug.projects.$projectId.edit.tsx">
        </Route>
        <Route path="new" file="routes/app.$organizationSlug.projects.new.tsx">
      </Route>
    </Route>
  </Route>
</Routes>

The way flat routes works is that it first builds up the route structure based on your filenames. Then it loops through the routes looking for the parent. It removes the last segment and looks for a match. If it finds it, then that's the parent. If not, it strips off the trailing segment again looking for a match, all the way to the root.

So let's take this set of routes. Notice how everything nests under each parent route because it finds a matching parent (when stripping off the leaf route).

const routeList = [
  'parent.tsx',
  'parent.some.nested.tsx',
  'parent.some.nested.route.tsx',
]

<Routes>
  <Route file="root.tsx">
    <Route path="parent" file="routes/parent.tsx">
      <Route path="some/nested" file="routes/parent.some.nested.tsx">
        <Route path="route" file="routes/parent.some.nested.route.tsx">
      </Route>
    </Route>
  </Route>
</Routes>

The only time you would need the trailing slash is if you want to base a route off a different parent than the what it would find based on the algorithm above. Here, you'd want to say the parent of parent.some.nested.route is parent, and not parent.some.nested (since it would match). By appending the _ to the child of the parent you want to attach to, it ensures it'll match the correct parent. Also note that it is correctly stripping the trailing slash from the path (so you shouldn't have issue anymore with params).

const routeList = [
  'parent.tsx',
  'parent.some.nested.tsx',
  'parent.some_.nested.route.tsx', // don't want this route under parent.some.nested
]

<Routes>
  <Route file="root.tsx">
    <Route path="parent" file="routes/parent.tsx">
      <Route path="some/nested" file="routes/parent.some.nested.tsx">
      <Route path="some/nested/route" file="routes/parent.some_.nested.route.tsx">
    </Route>
  </Route>
</Routes>

@alexluong
Copy link
Author

Thanks for the explanation. I totally got my routing wrong earlier. Here's what I had in mind

/app/$organizationSlug_/_projects
/app/$organizationSlug_/_projects/projects/new
/app/$organizationSlug_/_projects/projects/$projectId
/app/$organizationSlug_/_projects/projects/$projectId/edit
<Routes>
  <Route file="root.tsx">
    <Route path="app/:organizationSlug" file="routes/app.$organizationSlug.tsx">
      <Route path="edit" file="routes/app.$organizationSlug.edit.tsx" />
    </Route>
    <Route file="app/$organizationSlug_/_projects.tsx">
      <Route path="projects/:projectId" file="routes/app.$organizationSlug.projects.$projectId.tsx">
        <Route path="edit" file="routes/app.$organizationSlug.projects.$projectId.edit.tsx" />
      </Route>
      <Route path="projects/new" file="routes/app.$organizationSlug.projects.new.tsx" />
    </Route>
  </Route>
</Routes>

The reason for the pathless parent route is that I don't want to have the /app/:organizationSlug/projects as a valid URL.


Anyway, I appreciate you hoping on this really quickly. However, I'm seeing some issues after updating. It seems like any children route under the route with the trailing _ returns a 404 now.

Inspecting the routes with remix routes, I think the issue is the children path is constructed incorrectly.

        <Route path="organizations/$organizationId/salons/:salonId" file="routes/_main.admin.organizations.$organizationId_.salons.$salonId.tsx">
          <Route path="edit" file="routes/_main.admin.organizations.$organizationId_.salons.$salonId.edit.tsx" />
        </Route>
        <Route path="organizations/$organizationId/salons/new" file="routes/_main.admin.organizations.$organizationId_.salons.new.tsx" />

Notice the $ instead of : in the path.

Looking at the source code:

    if (segment.startsWith('_')) {
      // handle pathless route (not included in url)
      return url
    } else if (segment.endsWith('_')) {
      // handle parent override
      segment = segment.substring(0, segment.length - 1)
      url += '/' + segment
    } else if (['index', '_index'].some(name => segment === name)) {
      // handle index route
      if (!url.endsWith('/')) {
        url += '/'
      }
    } else if (segment.startsWith('$')) {
      // handle params
      segment = segment === '$' ? '*' : `:${segment.substring(1)}`
      url += '/' + segment
    } else {
      url += '/' + segment
    }

I think the problem is in this condition. The logic is not a straightforward "either or". A segment can start with _ or end with _ and still have $ in there. Also, it seems like this doesn't account for cases where there's a leading _ at the param, not sure if that's the intended behavior.

@kiliman
Copy link
Owner

kiliman commented May 31, 2022

Oops.. you're right. Param handling should still be processed. I'll remove all the elses.

kiliman added a commit that referenced this issue May 31, 2022
@kiliman
Copy link
Owner

kiliman commented May 31, 2022

const routeList = [
  'app.$organizationSlug_._projects.tsx',
  'app.$organizationSlug_._projects.projects.new.tsx',
  'app.$organizationSlug_._projects.projects.$projectId.tsx',
  'app.$organizationSlug_._projects.projects.$projectId.edit.tsx',
]
<Routes>
  <Route file="root.tsx">
    <Route path="app/:organizationSlug" file="routes/app.$organizationSlug_._projects.tsx">
      <Route path="projects/new" file="routes/app.$organizationSlug_._projects.projects.new.tsx">
      <Route path="projects/:projectId" file="routes/app.$organizationSlug_._projects.projects.$projectId.tsx">
        <Route path="edit" file="routes/app.$organizationSlug_._projects.projects.$projectId.edit.tsx">
      </Route>
    </Route>
  </Route>
</Routes>

@alexluong
Copy link
Author

Awesome! Can confirm everything is working as expected. Appreciate it!

@kiliman
Copy link
Owner

kiliman commented Jun 1, 2022

So what do you think of flat-routes? Do you feel it is more intuitive than the default routing?

@alexluong
Copy link
Author

I really enjoy it. I'm not sure if it's more intuitive, but I generally prefer the flat folder structure.

Also really like that I can now write more code inside the routes folder and have components and stuff inside each route. Previously when I was working with folder-based routing like Remix and Next.js, I generally avoid writing code in the pages and routes folder.

I'll continue building my project with flat-routes and will report back any issues or further thoughts!

@kiliman
Copy link
Owner

kiliman commented Jun 3, 2022

Thanks for the comments. I'd like to make a VS Code extension that will make it easier to create a flat route. This way you won't have to type in the prefixes. It is easier to create routes in the original way, since you just have to create a new file inside the folder.

Other than that, I agree with you about the "pros".

@alexluong
Copy link
Author

Gotcha. Just some personal opinions here:

It is easier to create routes in the original way, since you just have to create a new file inside the folder.

I don't agree with this notion because I tend to hide my folders when I work, but this is just my personal preference. I don't mind having to type out the full layout because it makes me internalize the layout tree and understand the impact of my new route.

I'd like to make a VS Code extension that will make it easier to create a flat route.

Personally I don't like adding more VS Code extension, especially GUI ones as I like to keep everything quite minimal in my setup. Again, just personal preference, so for me I won't find this as valuable. Maybe others will find more value in it.

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