vercel / vercel/next.js

Inconsistent Error Messaging / Handling in getStaticPaths

Open
#41,281 51 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

good first issue
Dominant language
JavaScript
Stars
142k
Forks
32.4k
Avg merge
2d 14h
Merged PRs (30d)
351

Description

Verify canary release
  • I verified that the issue exists in the latest Next.js canary release
Provide environment information
Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 21.0.1: Tue Sep 14 20:56:24 PDT 2021; root:xnu-8019.30.61~4/RELEASE_ARM64_T6000
Binaries:
  Node: 16.14.2
  npm: 8.5.0
  Yarn: 1.22.19
  pnpm: N/A
Relevant packages:
  next: 12.3.2-canary.22
  eslint-config-next: 12.3.1
  react: 18.2.0
  react-dom: 18.2.0
What browser are you using? (if relevant)

Google Chrome (Version 105.0.5195.125 (Official Build) (arm64))

How are you deploying your application? (if relevant)

Locally via next dev

Describe the Bug

As a preface, this is absolutely human error, but still a possibility to improve error handling and developer experience as I (at least I hope 😅) am probably not the first or last to make this simple oversight.

Additionally, I apologize if this is a duplicate. I did attempt to find an existing report via Google and Github issue search; but at 1.2k open issues, I can't be 100% sure I didn't just miss it.

When creating dynamic routes in combination with getStaticPaths and getStaticProps, error handling does not cover / wrap when a non-string value is supplied as the parameter to a dynamic route.

image

I was able to Google my error message and figure out relatively quickly that the issue was I forgot to convert my page number from pagination data to a string as the argument for the paths property of the return; but this was not immediately clear.

EDIT: This appears to be as simple as just adding a type check and error to the escapePathDelivers() in router utils, but I am suspicious of it being that easy so a second opinion before I open a PR would be appreciated.

Expected Behavior

Incompatible return output from getStaticPaths would ideally be handled in the same way existing error handling is done for other problems. For example, if you omit a square bracket from the file, eg [[...page].jsx, or supply an array to a single-parameter route or a string to a catch-all route, then it gives a plain text error like "Segment names may not start or end with extra brackets ('[...page')." or "A required parameter (page) was not provided as an array" where it's immediately clear how to fix the problem.

Link to reproduction

https://github.com/jodylecompte/min-repro-nextjs-static-paths

To Reproduce
  • Clone repo, install dependencies, and run dev server.
  • Navigate to route /articles/3
  • Error from screenshot above will be shown

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with escapePathDelivers() in the router utilities and reproduce the failure using the linked min-repro project by navigating to /articles/3. Compare this path-parameter error with the existing getStaticPaths validation messages; done means an invalid non-string parameter produces a clear actionable error instead of the current failure.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, next.js
Domain
web-dev
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.