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

feat: add utility for router params (#120) #158

Merged
merged 4 commits into from Aug 3, 2022
Merged

feat: add utility for router params (#120) #158

merged 4 commits into from Aug 3, 2022

Conversation

NozomuIkuta
Copy link
Member

Resolves #120

I added getRouterParams(event) utility function.
I also added useRouterParams(event) for it to be aligned with other API until the next braking change (may be v1?).

Comment on lines +13 to +16
export function getRouterParams (event: CompatibilityEvent): CompatibilityEvent['context'] {
// Fallback object needs to be returned in case router is not used (#149)
return event.context.params || {}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This comment is just for GitHub to add a reference link to #149.

@codecov
Copy link

codecov bot commented Aug 3, 2022

Codecov Report

Merging #158 (819d133) into main (272f883) will decrease coverage by 0.13%.
The diff coverage is 54.54%.

@@            Coverage Diff             @@
##             main     #158      +/-   ##
==========================================
- Coverage   65.53%   65.40%   -0.14%     
==========================================
  Files          14       14              
  Lines         911      922      +11     
  Branches      195      200       +5     
==========================================
+ Hits          597      603       +6     
  Misses        124      124              
- Partials      190      195       +5     
Impacted Files Coverage Δ
src/utils/request.ts 59.15% <54.54%> (-0.85%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@@ -10,6 +10,14 @@ export function getQuery (event: CompatibilityEvent) {
/** @deprecated Use `h3.getQuery` */
export const useQuery = getQuery

export function getRouterParams (event: CompatibilityEvent): CompatibilityEvent['context'] {
Copy link
Member

Choose a reason for hiding this comment

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

Can we please export it from src/router.ts or src/utils/router.ts?

@@ -53,6 +53,56 @@ describe('', () => {
})
})

describe('getRouterParams', () => {
Copy link
Member

Choose a reason for hiding this comment

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

Can we please move it to test/router.test.ts ?

@pi0
Copy link
Member

pi0 commented Aug 3, 2022

Nice addition! Would be nice to also introduce getRouterParam(event, name) :)

@NozomuIkuta
Copy link
Member Author

@pi0

I added getRouterParam(event, name) and useRouterParam(event, name), and reorganized unit tests.

@NozomuIkuta NozomuIkuta requested a review from pi0 August 3, 2022 18:29
}

/** @deprecated Use `h3.getRouterParam` */
export const useRouterParam = getRouterParam
Copy link
Member

Choose a reason for hiding this comment

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

We never introduce useRouter* utils. Then don't need to add and deprecate. deprecation of other utils was that they could be used in existing projects.

Copy link
Member Author

Choose a reason for hiding this comment

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

Deleted! 🙋‍♀️

819d133

Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

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

Amazing! Thank you!

@pi0 pi0 merged commit 4b83bdf into unjs:main Aug 3, 2022
@NozomuIkuta NozomuIkuta deleted the feat/add-utility-for-router-params branch August 3, 2022 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add useParam(event) like useQuery(event)
2 participants