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.

Sign up for GitHub

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

Jump to bottom

[legacy-framework] Change useParam / useParams to always return undefined for empty/invalid values instead of "" and NaN (patch) #1491

Merged
merged 5 commits into from
Nov 24, 2020
Merged

[legacy-framework] Change useParam / useParams to always return undefined for empty/invalid values instead of "" and NaN (patch) #1491

merged 5 commits into from
Nov 24, 2020

Conversation

KATT
Copy link
Collaborator

@KATT KATT commented Nov 20, 2020 •
edited
Loading

Related: blitz-js/legacy-framework#616 #1474

What are the changes and their implications?

  • Refactor useParam / useParams so that useParams does all the typecasting magic and useParam simply gets params[key]
  • useParam with number will never return NaN

Checklist.

  • Tests added for changes
  • PR submitted to blitzjs.com for any user facing changes

KATT added 2 commits November 18, 2020 12:26
@KATT
do parsing in useParams instead of useParam
6ef7272
@KATT
simplify
98d75b5 
# Conflicts:
#	packages/core/src/use-params.ts
KATT
KATT commented Nov 20, 2020
View reviewed changes
packages/core/src/use-params.ts Outdated Show resolved Hide resolved
@KATT KATT mentioned this pull request Nov 20, 2020
Merged
2 tasks
KATT
KATT commented Nov 20, 2020
View reviewed changes
packages/core/src/types.ts
@@ -64,6 +64,10 @@ export interface CancellablePromise<T> extends Promise<T> {

export type QueryFn = (...args: any) => Promise<any>

export type Dict<T> = Record<string, T | undefined>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shortcut to writing Record<string, string|undefined> etc

Iguanya
Iguanya approved these changes Nov 23, 2020
View reviewed changes
Copy link

@Iguanya Iguanya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well that's helpful

@KATT KATT marked this pull request as ready for review November 23, 2020 10:58
@KATT KATT requested a review from flybayer as a code owner November 23, 2020 10:58
@flybayer
Copy link
Member

Awesome, thanks @KATT!!

@flybayer flybayer changed the title Refactor useParam / useParams Change useParam / useParams to always return undefined for empty/invalid values instead of "" and NaN (patch) Nov 24, 2020
@flybayer flybayer added the 0 - <(^_^)> - merge it! ✌️ Kodiak automerge label Nov 24, 2020
@kodiakhq kodiakhq bot merged commit 1194ffd into blitz-js:canary Nov 24, 2020
@KATT KATT deleted the feature/return-strict-or-undefined branch November 24, 2020 15:30
@itsdillon itsdillon changed the title Change useParam / useParams to always return undefined for empty/invalid values instead of "" and NaN (patch) [legacy-framework] Change useParam / useParams to always return undefined for empty/invalid values instead of "" and NaN (patch) Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rayandrew rayandrew rayandrew left review comments

@dmzza dmzza dmzza left review comments

@Iguanya Iguanya Iguanya approved these changes

@flybayer flybayer flybayer approved these changes

Assignees
No one assigned
Labels
0 - <(^_^)> - merge it! ✌️ Kodiak automerge status/done
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@KATT @flybayer @dmzza @rayandrew @Iguanya