-
-
Notifications
You must be signed in to change notification settings - Fork 10
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: sorting search params before navigating #56
Conversation
🦋 Changeset detectedLatest commit: e907ec2 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 recent updates introduce sorting of query parameters to enhance cache hit rates for HTTP requests. By default, parameters are alphabetically sorted, and developers can customize this behavior or opt out entirely. Changes across several files ensure consistent handling of query parameters, including their declaration, storage, and testing. Changes
Assessment against linked issues
Poem
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 X ? TipsChat with CodeRabbit Bot (
|
✅ Deploy Preview for sveltekit-search-params ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 5
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- package.json
Files selected for processing (6)
- .gitignore (1 hunks)
- README.md (13 hunks)
- playground/src/routes/+page.svelte (2 hunks)
- playground/src/routes/queryparameters/+page.svelte (2 hunks)
- src/lib/sveltekit-search-params.ts (5 hunks)
- tests/index.test.ts (2 hunks)
Files skipped from review due to trivial changes (1)
- .gitignore
Additional comments: 22
README.md (9)
49-51: The import statement for
queryParam
has been updated to use single quotes, which is a minor stylistic change and aligns with the summary provided.100-103: The
queryParam
function now includes anencode
anddecode
option, which is consistent with the PR's objective to allow for custom sorting or disabling the sorting feature.120-124: The
defaultValue
option has been added to thequeryParam
function call, which is a new feature mentioned in the summary. This allows for a default value to be set for a query parameter.145-145: The use of helper functions for parameter types (
ssp.number()
) is demonstrated, which improves readability and usability as mentioned in the summary.160-160: Here, a default value is passed to the helper function, which is a new feature that aligns with the PR's objective to provide more control over query parameters.
175-175: The
queryParameters
function is used without arguments, which suggests that it will manage all query parameters without sorting, as per the new feature.226-228: The
queryParameters
function is now accepting an object that specifies which parameters should be managed, which is consistent with the PR's objective and the summary provided.310-310: The use of the
ssp.boolean(true)
helper function is consistent with the PR's objective to provide helper functions for common parameter types.404-419: The configuration options for
queryParam
andqueryParameters
are demonstrated, which includes the newsort
option. This aligns with the PR's objective to allow for better cache-ability by sorting query parameters.playground/src/routes/+page.svelte (2)
9-11: The implementation of the
arr_unordered
query parameter with thesort
option set tofalse
aligns with the PR's objective to introduce unsorted query parameters for better cache-ability.47-68: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [6-67]
Given the PR's objective and the changes made, it's important to ensure that the new feature is well-documented and that there are comprehensive tests covering the new sorting behavior. The author's concern about the change being potentially breaking should be addressed by thoroughly reviewing the impact on the public API and considering a major version release if necessary.
playground/src/routes/queryparameters/+page.svelte (3)
13-20: The implementation of
unordered_store
with thesort
property set tofalse
correctly aligns with the PR's objective to allow unsorted query parameters. This should enable thearr-unordered
parameter to be managed without affecting cache consistency.58-74: The addition of the button to add items to the
arr-unordered
array and the UI elements to display it are consistent with the existing pattern of the application. The use of the spread operator to trigger reactivity in Svelte is correct, ensuring the UI updates when the array changes.9-11: The existing store initialization does not show any changes related to the new sorting feature. It's important to ensure that documentation and examples are updated to guide users on how to use the new
sort
property with existing stores if this is a breaking change.src/lib/sveltekit-search-params.ts (6)
30-34: The addition of the
sort
property to theStoreOptions
interface is consistent with the PR's objective to introduce sorting of search parameters. This is a significant change that could impact existing consumers of these functions.158-161: The
queryParameters
function has been updated to include thesort
property with a default value oftrue
. This ensures that sorting is enabled by default, maintaining backward compatibility for existing consumers who do not specify thesort
option.246-249: The
queryParam
function has been updated to include thesort
property with a default value oftrue
. This change maintains backward compatibility for existing consumers who do not specify thesort
option.196-208: The conditional sorting logic using the
sort
property before updating the URL and pushing history is implemented correctly and aligns with the PR's objective to improve cache-ability by ensuring consistent order of query parameters.274-286: The conditional sorting logic in the
queryParam
function is consistent with thequeryParameters
function, correctly using thesort
property to ensure the order of query parameters before updating the URL and pushing history.193-211: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [30-286]
Given the changes to the function signatures and the introduction of a new feature that alters URL generation behavior, this PR introduces breaking changes. It would be prudent to release these changes as a major version update to adhere to semantic versioning principles.
tests/index.test.ts (2)
119-130: The test case 'parameters are kept in alphabetical order by default' correctly checks the URL search string to ensure parameters are sorted alphabetically. This aligns with the PR's objective to improve cache-ability by sorting query parameters.
272-282: The test case 'parameters are kept in alphabetical order by default' in the
queryParameters
describe block is similar to the one in thequeryParam
block. It checks the URL search string to ensure parameters are sorted alphabetically. This is consistent with the feature being tested and the PR's objective.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (2)
- .changeset/curvy-icons-protect.md (1 hunks)
- README.md (13 hunks)
Additional comments: 15
README.md (15)
49-51: The import statement and the usage of
queryParam
are updated to use single quotes, which is a minor stylistic change. This aligns with the PR's objective to standardize the usage examples in the documentation.67-67: The example correctly demonstrates the usage of
queryParam
for a single parameter. It's important to note that the PR's objective mentions sorting, but this simple case does not demonstrate the sorting feature.87-87: The example shows how to update the query parameter by assigning a new value to the store. This is consistent with the library's usage pattern.
101-103: The
encode
anddecode
functions are provided to handle the conversion of a number to a string and vice versa. This is a clear example of how to use these functions withqueryParam
.121-123: The
defaultValue
is introduced in the example, which is consistent with the PR's objective to allow setting default values for query parameters.145-145: This example uses a helper function
ssp.number()
to simplify the encoding and decoding process. This is a good demonstration of the library's helper functions.160-160: The example demonstrates how to use a helper function with a default value, which is a new feature mentioned in the PR's objective.
175-175: The
queryParameters
function is used without any options, which does not showcase the new sorting feature. It would be beneficial to include an example that demonstrates the sorting functionality.211-211: This example illustrates updating multiple query parameters at once by modifying the store object, which is consistent with the library's intended functionality.
227-227: The example shows how to specify expected query parameters, which is useful for initializing the store with predefined keys.
267-270: The
encode
anddecode
functions are used to handle boolean values, with adefaultValue
provided. This is a good example of more complex parameter handling.310-310: Here, the helper function
ssp.boolean(true)
is used to simplify the encoding and decoding of a boolean value with a default value, which aligns with the PR's objectives.396-396: The new
sort
option is introduced in the documentation, explaining its purpose and default behavior. This is a key part of the PR's objective and should be highlighted to inform users of the breaking change.406-410: The
debounceHistory
option is documented, which is not directly related to the sorting feature but is important for users to understand how to control the history behavior.417-417: The
pushHistory
option is mentioned, which allows users to control whether changes to the store result in new history entries. This is useful information that should be included in the documentation.
--- | ||
"sveltekit-search-params": minor | ||
--- | ||
|
||
feat: sorting search params before navigating |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the potential impact of the new sorting feature on existing users' workflows, it might be more appropriate to release this change as a major version update, not a minor one, to clearly communicate the breaking nature of the changes. Please consider the implications of this new feature on existing users and the semantic versioning guidelines.
Closes #51
This adds sorting before navigating to allow for better cache-ability. This is technically a breaking change (?) but i'm a bit unsure about the consequences that it might have so i might keep it as a minor
Summary by CodeRabbit
Documentation
queryParam
andqueryParameters
functions with improved examples.New Features
arr_unordered
that maintains an unordered list.unordered_store
to manage the state of unordered arrays in the UI.Refactor
queryParameters
andqueryParam
functions with asort
option to control the sorting of query parameters.Tests