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

Handle special props ref and key in path and search params #5537

Merged
merged 21 commits into from
Sep 4, 2022

Conversation

jtoar
Copy link
Contributor

@jtoar jtoar commented May 14, 2022

Closes #5497. When it comes to the router, it's react components all the way down. The router is a react component; routes are react components; pages are react components. We pass path and search parameters to pages as props, so that you can do things like...

const PostPage = ({ id }) => {
  return <PostCell id={id} />
}

(You can also get id from the params context via useParams, but destructuring it from props is more convenient.) When a path or search param is ref or key, there's an issue. If it's ref, the component throws since you can only pass the return of React.createRef or useRef to ref. With key, nothing happens, but the component consumes the prop and you don't get access to it. It's still accessible via useParams, but this isn't super obvious.

This PR stops the hard break that happens when ref is passed. It also stops the router from passing key.

@netlify
Copy link

netlify bot commented May 14, 2022

Deploy Preview for redwoodjs-docs canceled.

Name Link
🔨 Latest commit 9a3e15b
🔍 Latest deploy log https://app.netlify.com/sites/redwoodjs-docs/deploys/627f7d3ebed86e000806d9dc

@Tobbe
Copy link
Member

Tobbe commented Aug 30, 2022

Should we have a lint rule to make sure no one tries to have a route like /foo/{key}/bar/ or /books/{ref}/authors?

@jtoar
Copy link
Contributor Author

jtoar commented Aug 30, 2022

@Tobbe yeah that seems like the way to go. What should we do if the lint rule isn't heeded though? The router should probably fail in dev right? And in production? I still haven't thought it through

@jtoar
Copy link
Contributor Author

jtoar commented Aug 30, 2022

CI is failing because there's type error on allParams I was treating as a learning exercise

@jtoar jtoar changed the title Check for reserved names in search params Handle special props key and ref in path and search params Sep 3, 2022
@jtoar jtoar changed the title Handle special props key and ref in path and search params Handle special props ref and key in path and search params Sep 3, 2022
@jtoar jtoar force-pushed the ds-coerce-ref-search-param-to-referrer branch from ab0fc44 to 76020e1 Compare September 3, 2022 08:22
@jtoar jtoar marked this pull request as ready for review September 3, 2022 09:03
packages/router/src/util.ts Outdated Show resolved Hide resolved
jtoar and others added 3 commits September 4, 2022 10:37
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
@Tobbe Tobbe enabled auto-merge (squash) September 4, 2022 08:11
@Tobbe Tobbe merged commit 455bcaf into main Sep 4, 2022
@Tobbe Tobbe deleted the ds-coerce-ref-search-param-to-referrer branch September 4, 2022 08:36
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone Sep 4, 2022
jtoar added a commit that referenced this pull request Sep 5, 2022
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
jtoar added a commit that referenced this pull request Sep 8, 2022
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
dac09 added a commit to dac09/redwood that referenced this pull request Sep 9, 2022
* 'main' of github.com:redwoodjs/redwood:
  fix(deps): update dependency @graphql-yoga/common to v2.12.12 (redwoodjs#6349)
  fix(test-project): revert @redwoodjs/core to rc
  Update yarn.lock
  v2.2.4
  bugfix replace slash in tailwind config on windows (redwoodjs#6203)
  bugfix replace slash in tailwind config on windows (redwoodjs#6203)
  chore(deps): update dependency @testing-library/dom to v8.17.1 (redwoodjs#6351)
  Update yarn.lock
  Use try/catch to access unauthenticated (redwoodjs#6358)
  issue#5852 added windows fix for nodeFileTrace (redwoodjs#6325)
  Handle special props `ref` and `key` in path and search params (redwoodjs#5537)
  Use try/catch to access unauthenticated (redwoodjs#6358)
  feat(codemod): Add codemod to make relation resolvers partial (redwoodjs#6342)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

"Uncaught Error: Function components cannot have string refs" when adding a "?ref=xyz" param to the base url
2 participants