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

Added support for dynamic path params in client-only routes #119

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added support for dynamic path params in client-only routes #119

wants to merge 1 commit into from

Conversation

Hless
Copy link

@Hless Hless commented May 22, 2020

This is a proposed fix to support dynamic path parameters in routes.

Suppose you have a path called: /user/:userId . Version 0.3.x would not correctly pick up /user/14 for example, but would instead show a 404 page.

With this PR I have implemented redirection (from /user/14 to /en/user/14 for example), while replacing the named path parameters.

This PR does not (yet) include support for wildcard path matching using matchPath, but seeks to forgo that in favour of named path params. Currently this is because the changes in the PR make use of matchPath in order to match against paths without lang code prefix.

See issue #68, #93, #101

@justintemps
Copy link

This would be great to see! Any movement here?

@fabienheureux
Copy link

fabienheureux commented Jul 1, 2020

Hi @Hless, how did you implement your client only routes ?
When I define them in gatsby-node.js, your PR does not seem to fix the issue.

// gatsby-node.js
exports.createPage = async ({ actions }) => {
  actions.createPage({
    path: "/route/:param",
    matchPath: "/route/:param",
    component: path.resolve("./src/template/route.tsx")
  })
}

When I try to access /en/route/12/ for example, I get a 404.

@Hless
Copy link
Author

Hless commented Jul 9, 2020

Sorry for the late reply! Just came back from vacation. I have defined my routes as follows:

createPage({
    path: "blog/:slug",
    component: getTemplate("blog/blog"),
})

Big difference being that I omitted matchPath altogether. As stated in the description, there are several issues with matchPath and this plugin that should be addressed first.

Edit:
Excuse me, just read my own description more carefully. This PR uses matchPatch in order to match against languages, so named parameters for localized routes and matchPath functionality are mutually exclusive!

@jojobagings78
Copy link

jojobagings78 commented Jul 14, 2020

Sorry for the late reply! Just came back from vacation. I have defined my routes as follows:

createPage({
    path: "blog/:slug",
    component: getTemplate("blog/blog"),
})

Big difference being that I omitted matchPath altogether. As stated in the description, there are several issues with matchPath and this plugin that should be addressed first.

Edit:
Excuse me, just read my own description more carefully. This PR uses matchPatch in order to match against languages, so named parameters for localized routes and matchPath functionality are mutually exclusive!

Got same issue and not quite understanding how to work it placed in gatsby-node.js

exports.onCreatePage = async ({ page, actions }) => {
  const { createPage } = actions

  if (page.path.match(/^\/request/)) {
    page.matchPath = "/request/*"

    createPage(page)
  }
}

Then in my request.js file place

<Router>
     <ErrorPage default component={ErrorPage}/>
     <Pagerequest path="/request/:id" />
</Router>

Still though not working at all as every time I pass something for :id it either shows 404 when I include en in the url and if I remove the en in the url it removes the :id

@Hless
Copy link
Author

Hless commented Jul 15, 2020

Again, you should not use matchPath at all. Just define routes with named parameters in the path attribute in the createPages hook, and do not define matchPath. That means the PR in its current state is not compatible with named parameters of a dynamic router, like the one you seem to be using in your example. This PR is intended for server side rendered routes, but allows for dynamic parameters on runtime.

Quick example:

// gatsby-node.js
exports.createPages = async ({ graphql, actions }) => {
  const { createPage } = actions
   createPage({
      path: "/route/:param",
      component: path.resolve("./src/template/route.tsx")
  })
});

exports.onCreatePage = ({ page, actions: { createPage, deletePage } }) => {
  deletePage(page)
  createPage({
    ...page,
   // Whatever additional logic you might need here
  })
})

Component:

// src/template/route.tsx
interface MyRouteProps {
  param:string
}

const MyRoute: FC<MyRouteProps> = ({
  param 
}) => {
  // param is now equal to the :param parameter in the path
  // So for a route called /route/hello the prop param would be equal to 'hello'
});

@jojobagings78
Copy link

Again, you should not use matchPath at all. Just define routes with named parameters in the path attribute in the createPages hook, and do not define matchPath. That means the PR in its current state is not compatible with named parameters of a dynamic router, like the one you seem to be using in your example. This PR is intended for server side rendered routes, but allows for dynamic parameters on runtime.

Quick example:

// gatsby-node.js
exports.createPages = async ({ graphql, actions }) => {
  const { createPage } = actions
   createPage({
      path: "/route/:param",
      component: path.resolve("./src/template/route.tsx")
  })
});

exports.onCreatePage = ({ page, actions: { createPage, deletePage } }) => {
  deletePage(page)
  createPage({
    ...page,
   // Whatever additional logic you might need here
  })
})

Component:

// src/template/route.tsx
interface MyRouteProps {
  param:string
}

const MyRoute: FC<MyRouteProps> = ({
  param 
}) => {
  // param is now equal to the :param parameter in the path
  // So for a route called /route/hello the prop param would be equal to 'hello'
});

Apart from it will return
:param contains invalid WIN32 path characters.

Because you cant use :

@Hless
Copy link
Author

Hless commented Jul 15, 2020

Ah, that indeed looks like a bug for windows machines. Which is strange because Gatsby core supports this functionality:
gatsbyjs/gatsby#13965 (comment)

Which version of Gatsby do you use?

It could be mitigated by replacing : with _ (or another character that is safe for windows paths) during the conversion of path to matchPath in gatsy-node:

Feels kind of hacky though

@jojobagings78
Copy link

I am using
Gatsby CLI version: 2.8.19
Gatsby version: 2.18.12

Tried using your example and just nothing 404 error page

@jojobagings78
Copy link

jojobagings78 commented Jul 15, 2020

Ah, that indeed looks like a bug for windows machines. Which is strange because Gatsby core supports this functionality:
gatsbyjs/gatsby#13965 (comment)

Which version of Gatsby do you use?

It could be mitigated by replacing : with _ (or another character that is safe for windows paths) during the conversion of path to matchPath in gatsy-node:

Feels kind of hacky though

Got it to work with but problem is I change from en to say fr and the actual language does not change and stays on en even though its fr in the url.

exports.onCreatePage = async ({ page, actions }) => {
  const { path, matchPath, context } = page;
  const { createPage, deletePage } = actions;

  // Build client only paths for account
  if (path.match(/^\/request2\//)) {
    const { intl } = context;

    deletePage(page);
    createPage({
      ...page,
      matchPath: `/:locale/request2/*`,
      context: {
        ...context,
        intl: {
          ...intl,
          routed: true, // prevents auto redirection on these routes
        },
      },
    });
  }
}
<Router>
      <ErrorPage default component={ErrorPage}/>
      <Pagerequest path="/:locale/request2/:identifier" />
</Router>

@jojobagings78
Copy link

Ah, that indeed looks like a bug for windows machines. Which is strange because Gatsby core supports this functionality:
gatsbyjs/gatsby#13965 (comment)
Which version of Gatsby do you use?
It could be mitigated by replacing : with _ (or another character that is safe for windows paths) during the conversion of path to matchPath in gatsy-node:

Feels kind of hacky though

Got it to work with but problem is I change from en to say fr and the actual language does not change and stays on en even though its fr in the url.

exports.onCreatePage = async ({ page, actions }) => {
  const { path, matchPath, context } = page;
  const { createPage, deletePage } = actions;

  // Build client only paths for account
  if (path.match(/^\/request2\//)) {
    const { intl } = context;

    deletePage(page);
    createPage({
      ...page,
      matchPath: `/:locale/request2/*`,
      context: {
        ...context,
        intl: {
          ...intl,
          routed: true, // prevents auto redirection on these routes
        },
      },
    });
  }
}
<Router>
      <ErrorPage default component={ErrorPage}/>
      <Pagerequest path="/:locale/request2/:identifier" />
</Router>

Anyone got any clue ?

I was also able to get it to work but it was on en in url but showing in different language the last one in the array.

@Hless
Copy link
Author

Hless commented Jul 16, 2020

That is because that is still not the intended use case for this PR... I had the same problem using your code, that is why I made this PR. You are also not using the latest gatsby, might want to try that first?

But, the windows path problem made me realise that there is still work to do on this PR.

@jojobagings78 I would also like to kindly request not to post further problems with code that is not related to the workings of this pull request. You could try to open a different ticket for that.

@jojobagings78
Copy link

jojobagings78 commented Jul 24, 2020

Just cant seem to find a solution which shows how under developed this library is and everyone seems to be as lost as me.

@denplis
Copy link

denplis commented Aug 31, 2020

@jojobagings78 I've update this PR for my project and you could try it https://github.com/denplis/gatsby-plugin-intl/tree/matchpath

Installation:
npm install denplis/gatsby-plugin-intl#matchpath --save

Example of usage:

actions.createPage({
    path: "/route",
    matchPath: "/route/:param",
    component: path.resolve("./src/template/route.tsx")
})

....

if (template === 'invitation') {
    page.matchPath = '/invitation/:inviteId';
}

@wiltsu
Copy link

wiltsu commented Mar 28, 2021

Thank you @Hless
This fork fixed the issue where I had a page /order/:id and when the url for that page was set by a client without a locale prefix /en/order/:id/would lead to a 404

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

Successfully merging this pull request may close these issues.

6 participants