Fix the override setting to allow for false values #74
Conversation
🦋 Changeset detectedLatest commit: a87044a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe change involves a refinement in the logic for handling query parameters within a specific function. This adjustment shifts the focus from evaluating truthiness to explicitly checking for non-null values, thereby refining the control flow based on more precise value checks. This subtle yet significant change ensures a more accurate handling of query parameters and introduces a new functionality related to allowing false values in Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
❌ Deploy Preview for sveltekit-search-params failed.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/lib/sveltekit-search-params.ts (1 hunks)
Additional comments: 1
src/lib/sveltekit-search-params.ts (1)
- 382-382: The change from a truthy check to a non-null check (
$override != null) is a critical improvement for handling boolean values correctly, especially for values that default to false. This adjustment ensures that false values are not incorrectly treated as non-existent or undefined, which aligns with the PR's objective to fix the infinite redirect loop issue. By explicitly checking for null, the code now correctly differentiates between false and null values, allowing for the intended behavior when dealing with boolean query parameters.This change directly addresses the problem described in the PR objectives and is a safer approach given the explicit typing of
$overrideto be either a generic typeTornull. It's a good practice to use strict comparisons and checks in JavaScript to avoid unintended type coercions, which can lead to bugs, especially when dealing with values like0,'', orfalsethat are falsy but valid in certain contexts.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- .changeset/modern-llamas-bow.md (1 hunks)
Additional comments: 2
.changeset/modern-llamas-bow.md (2)
- 1-3: The metadata specifying a patch version change for
sveltekit-search-paramsis appropriate given the nature of the fix. This aligns well with semantic versioning practices.- 5-5: The description clearly and concisely explains the purpose of the change, which is to fix the override setting to allow for false values. This is crucial for understanding the context and impact of the patch.
Co-authored-by: Paolo Ricciuti <ricciutipaolo@gmail.com>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/lib/sveltekit-search-params.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/lib/sveltekit-search-params.ts
|
Hey @paoloricciuti 👋 Just following up here, I processed your suggestion a couple days ago, so should be all good now 🚀 |
Ugh sorry...thanks for pinging me |
Hi @paoloricciuti 👋
I found a small bug when using the package with a store value that was a boolean, and by default
false. I had this mount on a form submit after which a checkbox component would be mounted, mapped to a queryParam store that was of type boolean and by default false. It caused this weird infinite redirect loop, here is a little video of it:Screen.Recording.2024-03-04.at.15.47.37.mov
Was able to trace it down to an evaluation of
$overridewhich wouldn't work forfalsevalues. Changed it to do a comparison ofnullwhich should be a bit safer, as the type of this isT | null, so I assume that we want this path to run only when non-null.Summary by CodeRabbit