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

Avoid empty query values #3

Closed
zyxd opened this issue Sep 2, 2020 · 6 comments
Closed

Avoid empty query values #3

zyxd opened this issue Sep 2, 2020 · 6 comments
Assignees
Labels

Comments

@zyxd
Copy link

zyxd commented Sep 2, 2020

To avoid url like /path?page=&search= maybe better to check a value is not empty?
Or maybe it is better to add an option to 'clean' the query.

https://github.com/PaulMaly/svelte-pathfinder/blob/39aa6c177ebaade5d093501b715d9e8ef917a111/src/helpers.js#L52

@PaulMaly PaulMaly self-assigned this Sep 2, 2020
@PaulMaly
Copy link
Member

PaulMaly commented Oct 24, 2020

Returned to this proposal. I wonder, how we should figure out which value is not acceptable to the query param and which is not? For example, we've got a query state like this:

$query = {
   search: '',
   checked: false,
   selected: []
};

Should we remove the search param from this state? Is checked have a proper value at this moment? Seems both are falsy.

One more disadvantage is if we remove param from $query store we'll have to check all places to undefined. If we won't remove param from the store and just skip it in URL it will become some kind of inconsistency between state and URL. This inconsistency may cause us not to be able to recover falsy state for the specific value after page reload.

Any thoughts?

@zyxd
Copy link
Author

zyxd commented Nov 5, 2020

Maybe it makes sense not to remove any parameters from the state object, but hide it from string before history.pushState?

@PaulMaly
Copy link
Member

PaulMaly commented Nov 5, 2020

@zyxd The main issue is how to retrieve this state after page reload. So, this is exactly what I mean, our state is out of sync.

@zyxd
Copy link
Author

zyxd commented Nov 5, 2020

@PaulMaly I think it should be optional and disabled by default.

@PaulMaly
Copy link
Member

PaulMaly commented Nov 6, 2020

@zyxd Seems, this will cause us to lose information about the parameter type and be forced to check the value for existence in all places we use it:

{#if $query.filters} <!-- this check become necesery -->
   {#each $query.filters as filter}
       ...
   {/each}
{/if}

@PaulMaly
Copy link
Member

No updates here. Closed for now. Feel fee to open again if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants