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

Add useHashParam hook #6

Open
ThaNarie opened this issue Jan 7, 2022 · 3 comments
Open

Add useHashParam hook #6

ThaNarie opened this issue Jan 7, 2022 · 3 comments
Assignees
Labels
help wanted Extra attention is needed new-hook The addition of a new hook

Comments

@ThaNarie
Copy link
Contributor

ThaNarie commented Jan 7, 2022

Exists in the Muban Core Component Library.

Although that implementation works fine for a single hash param, working with multiple at the same time (i.e. updating two at once) will trigger two URL updates – meaning two history entries.

I wonder if we should extend this hook (or make another one) that supports multiple hash params as a reactive object, and clearly document when either one should be used?

@ThaNarie ThaNarie added help wanted Extra attention is needed new-hook The addition of a new hook labels Jan 7, 2022
@jyggiz
Copy link

jyggiz commented Nov 3, 2022

I could pick up this issue

@jyggiz
Copy link

jyggiz commented Nov 5, 2022

@ThaNarie I have a question in case we want to change our hook to support multiple parameters. If our parameters have, how is it better for us to pass the types of these parameters to the hook? And also what is the best way to redo the argument of the parse function to support multiple arguments?

Below, for clarity, I attach an example with the use of a hook with the current implementation:
const yearParm = useHashParam<number>('year', (value: string | null) => value ? parseInt(value, 10) : NaN, );

If we want, for example, to track 5 parameters with different types, then how to implement this?

I would not like to kill some part of the functionality of this hook, so I would like to suggest leaving this hook as it is and creating a new one to support multiple parameters. I am also open to suggestions and help!

@ThaNarie
Copy link
Contributor Author

ThaNarie commented Nov 7, 2022

Good question!

The only thing I can think of is creating an object:

const { year, month, setParams } = useHashParams<{ year: number; month: number }>({
  year:  (value: string | null) => value ? parseInt(value, 10) : NaN,
  month:  (value: string | null) => value ? parseInt(value, 10) : NaN,
});

// set single:
year.value = 2023;

// set multiple
setParams({
 month: 12,
 year: 2023,
});

This would support the single-use case as well, although slightly less elegant. So we could keep them both.

Downside of keeping both is that people might still use multiple single-ones instead of the multi-one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed new-hook The addition of a new hook
Projects
None yet
Development

No branches or pull requests

2 participants