-
Notifications
You must be signed in to change notification settings - Fork 11
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
adding typesafety for atomWithLocation #9
Comments
That's exactly why It can be too flexible DX-wise. Suggestions are welcome. |
@dai-shi
if we can setup something like this and drill the type it can be amazing dx
|
This is of course only one way to do it, but I will be honest I'm not quite sure how to drill the type into the search params.
|
If you need type safety (as I understand), just using It sounds like you may just want types (without safety) on search params. atomWithHash lies types (= no type safety) by default. On second look, your trial seems to follow the same approach. So, a broader discussion topic is, while |
(Hm, it can actually be a good idea. It would be nice to write an RFC in https://github.com/pmndrs/jotai/discussions for more attention.) |
Perfect, I think types are good enough for now maybe we can set it up in an extendable way (Zod/yup 🤔)
Overall I will definitely be up to creating RFC for this and might even give it a spin this weekend. |
Yeah, that's my concern too. At least, it's how So, this is a big discussion. We can go either way, or both, for location and hash. Nice to hear that you are interested in writing an RFC. Looking forward to it. Yeah, you can also play with the code. I actually want to re-implement |
@dai-shi |
So, for example, if someone does this? const atom1 = atomWithHash('theSameKey', 'foo')
const atom2 = atomWithHash('theSameKey', 'bar') It would cause a trouble. Or, do you mean two different keys? const atom3 = atomWithHash('fooKey', 'foo')
const atom4 = atomWithHash('barKey', 'bar') I think it's fine, because updates are sync. But just guessing. |
Well, current |
@dai-shi well I've done some research during the week and I think i found a few problems that this new atom raises atomWithSearchParam. |
Doesn't it break with current
👍 |
@dai-shi |
Yeah, and I'm actually interested in your trial to fix it. 😄 |
@dai-shi I've added a sandbox reproduction of a simple use-case (there are comments at every component) My attempts to fix this issues were fruitless (only solution is using NoSSR component or pre-initializing atom with data) |
@dai-shi you can also check atomWithSearch (it's very vague and not bulletproof yet, a few bugs exist) |
I'm not sure if I follow. there are two topics? a) a bug in |
@dai-shi
|
but i'm looking at the code.
Yeah, I didn't handle it. Good to know that there's a problem.
Ah, sounds like a bug. Do you know how to fix it?
Is it mostly similar to what we discussed as
Hm, we want something straightforward, or otherwise make things customizable. The implementation seems tricky, but that's not very important for now. export const userAtom = atomWithSearch('state', {
id: 3,
date: new Date().toISOString(),
}); Oh, does that mean |
@dai-shi
What should be done in this situation? |
@WeiShengv99 Can you open a new issue, if it's unrelated to this issue? (I'm not sure if I understand "pages". A stackblitz reproduction might be helpful.) |
Hey, thanks for the hard work you have been putting into this new component,
I was wondering if you ever thought about adding type safety to atomWithLocation and/or serialization
We currently use Jotai and Jotai-location on our production app (NextJS), and it's sometimes a bit confusing to remember all the query keys if it needs to be shared across multiple components.
We were wondering if it would be possible for the feature to have something like this:
It might be out of the scope of this library but it just makes the DX much better.
Thanks again!
P.S
some resources:
https://github.com/pbeshai/use-query-params
https://github.com/amannn/next-query-params
The text was updated successfully, but these errors were encountered: