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

Defining multiple routes with same route module #1898

Closed
gijsroge opened this issue Feb 11, 2022 · 16 comments · Fixed by #3970
Closed

Defining multiple routes with same route module #1898

gijsroge opened this issue Feb 11, 2022 · 16 comments · Fixed by #3970
Labels
bug Something isn't working feat:routing

Comments

@gijsroge
Copy link

What version of Remix are you using?

1.1.3

Steps to Reproduce

As explained in react router v6 https://reactrouter.com/docs/en/v6/faq#what-happened-to-regexp-routes-paths to create optional parameters you can define multiple paths with the same route module file.

But this doesn't work in Remix. The last defined route overrides the previous one.

run npx create-remix@latest

remix.config.js

module.exports = {
  appDirectory: "app",
  assetsBuildDirectory: "public/build",
  devServerPort: 8002,
  publicPath: "/build/",
  serverBuildDirectory: "build",
  ignoredRouteFiles: [".*"],
  routes(defineRoutes) {
    return defineRoutes(route => {
      route("/user/:id", "routes/index.tsx");
      route("/user", "routes/index.tsx");
    });
  }
};

Expected Behavior

Both user/:id & user/ should render routes/index.tsx

Actual Behavior

only user/ resolves, user/:id returns a 404.

@gijsroge gijsroge added the bug Something isn't working label Feb 11, 2022
@jacob-ebey
Copy link
Member

This is something I'd like as well, the issue currently is we derive the route ID base on the file-path of the module, meaning the same module can't be shared for multiple route definitions. One thing I've considered is allowing another param to the route() method that accepts a unique user provided ID instead of deriving it from the module filepath.

@yomeshgupta
Copy link
Contributor

I had somewhat similar issue while working on https://devtools.tech. I wanted to show the same file/component on two different routes. As @jacob-ebey mentioned the route ID is based on file path so you cannot include the same file twice.

One solution I could think was abstract out the contents into a third component and import it into two different routes.

// Component.js
...

export const links;
export const meta;
export default Component;
// route /blog

import Component, { links as importedLinks, meta as importedMeta } from './Component';

export const links = importedLinks;
export const meta = importedMeta;

// we can directly export the component or wrap it if we want to pass or do anything extra.
export default Component;
// route /resource

import Component, { links as importedLinks, meta as importedMeta } from './Component';

export const links = importedLinks;
export const meta = importedMeta;

// we can directly export the component or wrap it if we want to pass or do anything extra.
export default Component;

I even tried defining a route in the routes folder like [blog|resource].jsx but as far as I remember that didn't work. 😅

@gijsroge
Copy link
Author

gijsroge commented Feb 12, 2022

@jacob-ebey can't the key be the path you defined in the route() method? I don't know the underlying stack so I have absolutely no idea if that would cause any issues :D

@jacob-ebey
Copy link
Member

Paths will not work. Nested, layout, etc routes all can have the same path.

@gijsroge
Copy link
Author

Yeah, just noticed when running the tests, an optional user specified key in the options sounds like great way to solve this.

Is this worth a PR from me?

@jacob-ebey
Copy link
Member

@gijsroge if you'd like to explore this and do a PR I'll follow up. If you are in the discord ping me there if you have any Q's.

@gijsroge
Copy link
Author

gijsroge commented Feb 22, 2022

@jacob-ebey sorry I can't seem to find the time, also admitting I struggled with figuring this out.

@kiliman
Copy link
Collaborator

kiliman commented Feb 22, 2022

Perhaps the key can be ${path}:${filename}. That must be a unique combination. Otherwise you'll have to create a bunch of extra files for no reason and kind of defeats the route config option.

@mzaatar
Copy link

mzaatar commented Apr 13, 2022

following this as I have the same issue here

@runmoore
Copy link
Contributor

runmoore commented May 3, 2022

One use-case for this that I see as being really valuable is translated urls. It's mentioned in this discussion, but to summarise:

Having code located in the repo at /routes/about but rather than just supporting one url, we are able so support the url translated into many languages:

en-US : path = /about
fr-FR : path = /a-propos

Currently the way I've found that could support this is as part of a pre-build step. Before running remix build, all routes are duplicated based on translations. I doubt this is a very scalable solution though and would be interested to know if a feature like this could make it into Remix

@kiliman
Copy link
Collaborator

kiliman commented May 4, 2022

I played around with the ability to add a namespace to routes via defineRoutes => route. I haven't tested this thoroughly, but it will be part of my remix-mount-routes and remix-flat-routes packages in a future update.

@gijsroge
Copy link
Author

@kiliman any reason why you are not attempting a PR for this rather than a third party package?

@kiliman
Copy link
Collaborator

kiliman commented May 10, 2022

I like to test out ideas first to see what works before trying to change the core.

See my remix-flat-routes package that gives you a new convention for defining routes (based on Ryan's gist).

https://github.com/kiliman/remix-flat-routes

remix-mount-routes was my first attempt as customizing routes. The API will likely change since I learned a lot with the flat routes package.

@vsnig
Copy link

vsnig commented May 21, 2022

I think there also should be an option to pass params to route modules that will be available in loader (may be merged with :params).
This is to be able to implement this pattern (taken from RRv6 FAQ)

<Routes>
  <Route path="en" element={<Lang lang="en" />}/>
  <Route path="es" element={<Lang lang="es" />}/>
  <Route path="fr" element={<Lang lang="fr" />}/>
</Routes>

@lucasferreira
Copy link
Contributor

Hi guys,

I created a PR (#3970) trying to fix this problem, if someone could take a look ;)

My ideia it's to pass a optional route ID when we have a multiple route conflict, something like this:

module.exports = {
  appDirectory: "app",
  assetsBuildDirectory: "public/build",
  devServerPort: 8002,
  publicPath: "/build/",
  serverBuildDirectory: "build",
  ignoredRouteFiles: [".*"],
  routes(defineRoutes) {
    return defineRoutes(route => {
      route("/user/:id", "routes/index.tsx", { id: "user-by-id" });
      route("/user", "routes/index.tsx");
    });
  }
};

@brophdawg11 brophdawg11 added the awaiting release This issue has been fixed and will be released soon label Dec 1, 2022
@brophdawg11
Copy link
Contributor

This is merged to dev and should be released in 1.9.0 next week or so 👍

@brophdawg11 brophdawg11 removed the awaiting release This issue has been fixed and will be released soon label Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feat:routing
Projects
None yet
10 participants