gvergnaud / nextjs-dynamic-routes

[Deprecated] Super simple way to create dynamic routes with Next.js
MIT License
140 stars 7 forks source link

[Feature request] Path param prop checking #19

Closed tranhl closed 2 years ago

tranhl commented 5 years ago

Currently, the Link component does not check that you have correctly passed any expected path params:

import React from 'react'
import { Link } from '../routes'

export default () => (
  <>
    <Link route="character" id="1"><a>Luke Skywalker</a></Link>
    <Link route="character"><a>C-3PO</a></Link> {/* Missing 'id' prop won't throw an error */}
  </>
)

I think that an error should be thrown to if the developer forgets to pass in the correct prop to save time debugging later down the track. In the above example, the id prop should be checked, and if it isn't passed into the Link component, an error should be thrown.

gvergnaud commented 5 years ago

Hey, This is indeed a scenario I didn't take into consideration. I agree we should do something, but actually I don't think we should throw if the route doesn't have all of its params. Params are evaluated during runtime, which could make your app break in unexpected ways. Instead we should definitely warn you in the console that a param is missing in dev, kinda like what react is doing. I might tackle this in the next version

lydell commented 5 years ago

Very much related: #16, especially https://github.com/gvergnaud/nextjs-dynamic-routes/issues/16#issuecomment-455151226

(@tranhl Shameless plug – if you need something like this today there's next-minimal-routes :sweat_smile:)

tranhl commented 2 years ago

Closing due to staleness.