Skip to content

Conversation

@suinkimme
Copy link

📝 Key Changes

The example in use-page-state-readability.md incorrectly passed an object to the useQueryParam setter:

_setCardId({ cardId }, "replaceIn");

This PR updates the example to use the correct usage:

_setCardId(cardId, "replaceIn");

This fixes misleading documentation and aligns the example with the expected API of useQueryParam.

🖼️ Before and After Comparison

Before After
_setCardId({ cardId }, "replaceIn"); _setCardId(cardId, "replaceIn");

This PR has been re-submitted from a new branch with the corrected changes.
The previous PR (690) has been closed.
Please refer to this updated PR for the latest version.

@vercel
Copy link

vercel bot commented Nov 27, 2025

@suinkimme is attempting to deploy a commit to the Toss Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

@kss2002 kss2002 Nov 28, 2025

Choose a reason for hiding this comment

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

hello 👋
You changed the call _setCardId from object to primitive

// previous
_setCardId({ cardId }, "replaceIn");

// updated
_setCardId(cardId, "replaceIn");

This may change the shape of the query parameter and could affect existing behavior or consumers that expect the object form.

Can you confirm that this change is intentional and aligned with how useQueryParam + NumberParam are expected to work?

I think updating the dependency array to include _setCardId is a good improvement!

Thank you :)

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the review and for pointing this out!

In this case, the change was intentional.
Since useQueryParam("cardId", NumberParam) is used, the setter returned by useQueryParam expects a primitive number, not an object. Passing { cardId } was causing a mismatch with how NumberParam serializes the value, and could result in an incorrect query parameter shape.

Therefore, _setCardId(cardId, "replaceIn") is the correct usage aligned with the expected behavior of useQueryParam + NumberParam.

Thanks again for the thoughtful review!

#76 (comment)

If I’m mistaken in any way, please don’t hesitate to let me know!

Copy link

Choose a reason for hiding this comment

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

Thank you for your answer.

I wrote a review considering the influence of other documents.
I think it's a good contribution given that it's a deliberate change.

Thank you :)

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.

2 participants