-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
docs: Add documentation for updating multiple parameters at once #106
Conversation
|
WalkthroughThe changes encompass updates to documentation and code formatting across several files. Notably, the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Store
participant Navigation
User->>Store: Update parameters
Store->>Store: Batch update using update()
Store->>Navigation: Navigate with updated params
Assessment against linked issues
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ 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.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (4)
README.md (4)
135-135
: Add a comma for clarity.The sentence can be improved by adding a comma after "server".
- You can't run `goto` on the server so if the page is server side rendered it will still have the null value (this is to say don't rely on the assumption that the store will always be not null). + You can't run `goto` on the server, so if the page is server side rendered it will still have the null value (this is to say don't rely on the assumption that the store will always be not null).Tools
LanguageTool
[uncategorized] ~135-~135: Possible missing comma found.
Context: ...rning** > > You can't rungoto
on the server so if the page is server side rendered ...(AI_HYDRA_LEO_MISSING_COMMA)
220-220
: Add a comma for clarity.The sentence can be improved by adding a comma after "at once".
- When you need to update multiple different parameters at once you will want to use the `update()` method to update the store to prevent multiple calls of `goto()`. + When you need to update multiple different parameters at once, you will want to use the `update()` method to update the store to prevent multiple calls of `goto()`.Tools
LanguageTool
[uncategorized] ~220-~220: Possible missing comma found.
Context: ...update multiple different parameters at once you will want to use theupdate()
met...(AI_HYDRA_LEO_MISSING_COMMA)
224-237
: Replace hard tabs with spaces.Hard tabs should be replaced with spaces for consistency and readability.
- import { queryParameters } from 'sveltekit-search-params'; + import { queryParameters } from 'sveltekit-search-params'; - const store = queryParameters(); + const store = queryParameters(); - const update = (value) => { + const update = (value) => { - store.update((v) => { + store.update((v) => { - return { + return { - ...v, + ...v, - from: value.from, + from: value.from, - to: value.to, + to: value.to, - period: value.period, + period: value.period, - }; + }; - }); + }); - }; + };Tools
Markdownlint
224-224: Column: 1
Hard tabs(MD010, no-hard-tabs)
226-226: Column: 1
Hard tabs(MD010, no-hard-tabs)
228-228: Column: 1
Hard tabs(MD010, no-hard-tabs)
229-229: Column: 1
Hard tabs(MD010, no-hard-tabs)
230-230: Column: 1
Hard tabs(MD010, no-hard-tabs)
231-231: Column: 1
Hard tabs(MD010, no-hard-tabs)
232-232: Column: 1
Hard tabs(MD010, no-hard-tabs)
233-233: Column: 1
Hard tabs(MD010, no-hard-tabs)
234-234: Column: 1
Hard tabs(MD010, no-hard-tabs)
235-235: Column: 1
Hard tabs(MD010, no-hard-tabs)
236-236: Column: 1
Hard tabs(MD010, no-hard-tabs)
237-237: Column: 1
Hard tabs(MD010, no-hard-tabs)
245-245
: Add a comma for clarity.The sentence can be improved by adding a comma after "store".
- If you don't use the update and instead individually assign each property on the store you may get `sveltekit 'client.js Uncaught (in promise) Error: navigation aborted'` errors. + If you don't use the update and instead individually assign each property on the store, you may get `sveltekit 'client.js Uncaught (in promise) Error: navigation aborted'` errors.Tools
LanguageTool
[uncategorized] ~245-~245: Possible missing comma found.
Context: ...ndividually assign each property on the store you may get `sveltekit 'client.js Uncau...(AI_HYDRA_LEO_MISSING_COMMA)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
playground/pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
Files selected for processing (6)
- CHANGELOG.md (1 hunks)
- README.md (2 hunks)
- playground/src/routes/+layout.svelte (1 hunks)
- src/lib/lz-string/index.js (1 hunks)
- src/routes/+page.svelte (1 hunks)
- src/routes/BrowserWindow.svelte (1 hunks)
Files skipped from review due to trivial changes (4)
- playground/src/routes/+layout.svelte
- src/lib/lz-string/index.js
- src/routes/+page.svelte
- src/routes/BrowserWindow.svelte
Additional context used
LanguageTool
CHANGELOG.md
[uncategorized] ~42-~42: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... - fde7148: fix: rework how defaults works ## 1.1.0 ### Minor Changes - 7a99c...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~90-~90: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...12 ### Patch Changes - 901d9c7: Add client side check before invoking goto ## 1.0.11 ...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~96-~96: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...7: removed changesets as dependency and add it as dev dependency ## 1.0.10 ### Pa...(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)
README.md
[uncategorized] ~135-~135: Possible missing comma found.
Context: ...rning** > > You can't rungoto
on the server so if the page is server side rendered ...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~220-~220: Possible missing comma found.
Context: ...update multiple different parameters at once you will want to use theupdate()
met...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~245-~245: Possible missing comma found.
Context: ...ndividually assign each property on the store you may get `sveltekit 'client.js Uncau...(AI_HYDRA_LEO_MISSING_COMMA)
Markdownlint
README.md
224-224: Column: 1
Hard tabs(MD010, no-hard-tabs)
226-226: Column: 1
Hard tabs(MD010, no-hard-tabs)
228-228: Column: 1
Hard tabs(MD010, no-hard-tabs)
229-229: Column: 1
Hard tabs(MD010, no-hard-tabs)
230-230: Column: 1
Hard tabs(MD010, no-hard-tabs)
231-231: Column: 1
Hard tabs(MD010, no-hard-tabs)
232-232: Column: 1
Hard tabs(MD010, no-hard-tabs)
233-233: Column: 1
Hard tabs(MD010, no-hard-tabs)
234-234: Column: 1
Hard tabs(MD010, no-hard-tabs)
235-235: Column: 1
Hard tabs(MD010, no-hard-tabs)
236-236: Column: 1
Hard tabs(MD010, no-hard-tabs)
237-237: Column: 1
Hard tabs(MD010, no-hard-tabs)
Additional comments not posted (8)
CHANGELOG.md (6)
7-7
: LGTM!The formatting change is approved.
11-11
: LGTM!The formatting change is approved.
17-17
: LGTM!The formatting change is approved.
23-23
: LGTM!The formatting change is approved.
29-29
: LGTM!The formatting change is approved.
35-36
: LGTM!The formatting change is approved.
README.md (2)
218-241
: LGTM!The new section about updating multiple parameters at once is clear and provides valuable guidance.
Tools
LanguageTool
[uncategorized] ~220-~220: Possible missing comma found.
Context: ...update multiple different parameters at once you will want to use theupdate()
met...(AI_HYDRA_LEO_MISSING_COMMA)
Markdownlint
224-224: Column: 1
Hard tabs(MD010, no-hard-tabs)
226-226: Column: 1
Hard tabs(MD010, no-hard-tabs)
228-228: Column: 1
Hard tabs(MD010, no-hard-tabs)
229-229: Column: 1
Hard tabs(MD010, no-hard-tabs)
230-230: Column: 1
Hard tabs(MD010, no-hard-tabs)
231-231: Column: 1
Hard tabs(MD010, no-hard-tabs)
232-232: Column: 1
Hard tabs(MD010, no-hard-tabs)
233-233: Column: 1
Hard tabs(MD010, no-hard-tabs)
234-234: Column: 1
Hard tabs(MD010, no-hard-tabs)
235-235: Column: 1
Hard tabs(MD010, no-hard-tabs)
236-236: Column: 1
Hard tabs(MD010, no-hard-tabs)
237-237: Column: 1
Hard tabs(MD010, no-hard-tabs)
243-246
: LGTM!The warning about individually assigning properties on the store is clear and provides valuable guidance.
Tools
LanguageTool
[uncategorized] ~245-~245: Possible missing comma found.
Context: ...ndividually assign each property on the store you may get `sveltekit 'client.js Uncau...(AI_HYDRA_LEO_MISSING_COMMA)
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.
Thanks for the contribution...i've left a small comment. Also a lot of those fixes are just some prettier misconfiguration, can you revert those?
@@ -215,6 +215,35 @@ Just like with the single parameter case you can just update the store and the U | |||
|
|||
writing in the input will update the state and the URL at the same time. | |||
|
|||
### Updating multiple parameters at once | |||
|
|||
When you need to update multiple different parameters at once you will want to use the `update()` method to update the store to prevent multiple calls of `goto()`. |
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.
This only happens when using queryParameters
tho...can you please specify that in the documentation?
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.
Would it not happen if you had multiple queryParams? Doesn't it call goto the same way?
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.
If you update multiple queryParam
store in a single function it will wait a macrotask before navigating...unfortunately this is not really possible with queryParameters
but you have a very nice solution for this.
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.
Ah okay got it
Yeah sorry I just installed and ran format at the end. |
I will just close this and reopen it to git rid of the extra changes since its easier |
Adds guidance in the README on how to handle updating multiple parameters at once.
Summary by CodeRabbit