-
Notifications
You must be signed in to change notification settings - Fork 150
createUniqueId
can generate clashing ids
#263
Comments
This might be a misunderstanding. The id is only valid for a position inside the shared component tree. Inside the same position, you should always get the same id, both in the client and on the server. |
@atk thanks for chiming in. I am still not sure I quite understand what the purpose of the helper is. If I understand you correctly it's not meant to be a replacement for Should one ever call this outside of a components scope? |
It's meant to provide hydration IDs. You can use it for other things as well, but not for multiple unique IDs within the same component. And you're right, it's really underdocumented. |
Hm, we called it inside a helper function not related to components at all and got an id clash - so its probably best to not use it in that way? Should I keep this issue open for reference, or should I create another one for improving the documentation somewhere? :) |
That's for the core maintainers to decide. Maybe we could add an optional parameter to add a general id functionality to this function, I'm not sure. It's certainly a valid use case. |
I'm not quite clear on the purpose you are looking for here but looking at nanoid this is nothing like that. The idea of this is to create a stable ID for hydration or create an id for like labels for form fields etc.. It is very similar in purpose to: https://react.dev/reference/react/useId The id is not meant to be unique for all time but for any specific session. So yeah this seems like a documentation issue. I can move the issue to the solid-docs project. |
Am I correct to assume it's safe to use In other words, the following pattern is possible right? const marqueeId = createUniqueId()
onMount(() => {
const marquee = document.getElementById(marqueeId)
})
return (
<div id={marqueeId} />
) |
Describe the bug
When using
createUniqueId
and persisting and loading data, clashing ids might be generated. This can lead to corrupted/missing data and I would therefore consider this to be critical behavior. (all tested with client side solid only)I replaced calls to
nanoid
withcreateUniqueId
in my app under the impression that I would need one less dependency, because solid just brings it with its api.I am not sure if this is intended behavior or a bug. But if it is intended it would probably make sense to write a warning into the docs and also go into a bit more detail on what its purpose actually is. In my opinion this can easily lead to deter people from an otherwise amazing library (solidjs). I am lucky we figured this out in an internal app, where this wasn't super critical. But if this would've been a customer project and we had lost data due to it (apart from backup strategies) I would probaly think twice before using solid again.
This is just hypothetical of course - I love solid and the work you folks are doing 😉❤️
Your Example Website or App
Steps to Reproduce the Bug or Issue
I am not quite sure what the best way to reproduce this would be, but as far as I can tell in our case we ran code that called
createUniqueId
and persisted the created data. Later on we restarted our computer and created another entry which was created with the same id, replacing the content inside the store we were using.Expected behavior
As a user reading the docs I expected
createUniqueId
to be actually unique and not create clashing ids. It would also be nice to be able to control the length of the generated ids.Screenshots or Videos
No response
Platform
Additional context
No response
The text was updated successfully, but these errors were encountered: