Skip to content

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

Merged
Tobbe merged 21 commits intomainfrom
ds-coerce-ref-search-param-to-referrer
Sep 4, 2022
Merged

Handle special props ref and key in path and search params#5537
Tobbe merged 21 commits intomainfrom
ds-coerce-ref-search-param-to-referrer

Conversation

@jtoar
Copy link
Copy Markdown
Contributor

@jtoar jtoar commented May 14, 2022 •
edited
Loading

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.

@jtoar jtoar self-assigned this May 14, 2022
@netlify
Copy link
Copy Markdown

netlify Bot commented May 14, 2022 •
edited
Loading

✅ Deploy Preview for redwoodjs-docs canceled.

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

@jtoar jtoar mentioned this pull request Aug 29, 2022
Closed
@jtoar jtoar force-pushed the ds-coerce-ref-search-param-to-referrer branch from 9a3e15b to b8dd4e3 Compare August 29, 2022 12:44
Tobbe
Tobbe reviewed Aug 30, 2022
View reviewed changes
Comment thread packages/router/src/router.tsx Outdated
@Tobbe
Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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
Tobbe
Tobbe reviewed Sep 3, 2022
View reviewed changes
Comment thread packages/router/src/util.ts Outdated
Tobbe
Tobbe reviewed Sep 3, 2022
View reviewed changes
Comment thread packages/router/src/__tests__/util.test.ts Outdated
Tobbe
Tobbe reviewed Sep 3, 2022
View reviewed changes
Comment thread packages/router/src/__tests__/util.test.ts Outdated
@jtoar jtoar mentioned this pull request Sep 4, 2022
Closed
@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
@jtoar @Tobbe
Handle special props ref and key in path and search params (#5537)
21f4690 
Co-authored-by: Tobbe Lundberg <[email protected]>
jtoar added a commit that referenced this pull request Sep 8, 2022
@jtoar @Tobbe
Handle special props ref and key in path and search params (#5537)
5195bff 
Co-authored-by: Tobbe Lundberg <[email protected]>
dac09 added a commit to dac09/redwood that referenced this pull request Sep 9, 2022
@dac09
Merge branch 'main' of github.com:redwoodjs/redwood into try/tsd-tests
db82751 
* '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

Reviewers

1 more reviewer

@Tobbe Tobbe Tobbe left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@jtoar jtoar

Labels

release:fix This PR is a fix topic/router-&-navigation

Projects

No open projects
Main
Status: Done

Milestone

v2.2.4

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

@jtoar @Tobbe