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

Incoming request URL is not updated to include dynamic route params #35

Closed
GregBrimble opened this issue Nov 28, 2022 · 4 comments · Fixed by #87
Closed

Incoming request URL is not updated to include dynamic route params #35

GregBrimble opened this issue Nov 28, 2022 · 4 comments · Fixed by #87

Comments

@GregBrimble
Copy link
Member

GregBrimble commented Nov 28, 2022

Next.js attaches dynamic route params as query params. In normal routes, developers usually grab these values from the params object:

// pages/blog/[slug].ts

export async function getServerSideProps({ params }) {
  // params.slug = 'a-blog-slug'
}

But in Edge API routes, there is no such mechanism. In order to grab these values, you have to look at the incoming request URL:

// pages/api/blog/[slug].ts

export default function handler(request) {
  const url = new URL(request.url)
  url.searchParams.get('slug')
  // should be 'a-blog-slug' but is currently null
}

We should pass these handling functions the finalized URL which includes any dynamic route params in the query string.

@nakedible
Copy link

I got bitten by this. I thought my dynamic routes would work as is with this setup, but apparently not.

@RyleaStark
Copy link

export async function onRequestGet(event: Event): Promise<Response> {
  if (event.request.method === "GET") {
    return await myFunction(event.request);
  } else {
    console.log("Unauthorized request: Only GET requests are allowed");
    return new Response(JSON.stringify(["401", "Unauthorized"]), {
      status: 401,
      headers: {
        "Content-Type": "application/json",
      },
    });
  }
}
async function myFunction(request: Request): Promise<Response> {
  console.log("Function hit.");
  const params = new URLSearchParams(request.url.split('?')[1]);
  const creator = params.get('creator');
  const guid = params.get('guid');

  console.log({creator, guid});

  return new Response(JSON.stringify(["200", creator, guid]), {
      status: 200,
      headers: {
        "Content-Type": "application/json",
      },
   });
}

I had success with the above.

@nakedible
Copy link

Unfortunately, I just have pages which have dynamic routes - and those should be given in "params" to the functions.

Some pointers to help fix this...

The middleware-manifest.json that is being consulted to get the function matchers contains named groups already:

      "matchers": [
        {
          "regexp": "^/(?<id>[^/]+?)$"
        }
      ],

Hence this part in the actual worker:
https://github.com/cloudflare/next-on-pages/blob/main/templates/_worker.js/index.ts#L143-L161
just needs to be modified to keep the result of the regexp match operation, and place result.groups as context.params before invoking the handler.

At least, that is my guess what is sufficient.

@nakedible
Copy link

Apparently this is closer to my problem, and much closer also to the correct fix: #32

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 a pull request may close this issue.

3 participants