solid-docs icon indicating copy to clipboard operation
solid-docs copied to clipboard

`createUniqueId` can generate clashing ids

Open on3iro opened this issue 1 year ago • 7 comments

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 with createUniqueId 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

  • OS: OSX
  • Browser: Tauri Application
  • Version: Tauri-cli 1.4, Solidjs 1.7.7

Additional context

No response

on3iro avatar Jul 04 '23 17:07 on3iro

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 avatar Jul 04 '23 19:07 atk

@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 nanoid and the likes to create unique ids for data, but only for elements in the component tree. Is that correct? If so I would still argue that it would be really really helpful to better reflect that in the docs, because I assume that I am not the only one being mislead by its name thinking that it is a general purpose helper function :)

Should one ever call this outside of a components scope?

on3iro avatar Jul 04 '23 19:07 on3iro

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.

atk avatar Jul 04 '23 19:07 atk

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? :)

on3iro avatar Jul 04 '23 19:07 on3iro

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.

atk avatar Jul 04 '23 19:07 atk

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.

ryansolid avatar Jul 05 '23 19:07 ryansolid

Closing stale issues - if this is still an issue with the new docs when they're released, please file another issue!

LadyBluenotes avatar Nov 08 '23 18:11 LadyBluenotes

Am I correct to assume it's safe to use createUniqueId as a way to generate non-clashing ids?

In other words, the following pattern is possible right?

const marqueeId = createUniqueId()

onMount(() => {
  const marquee = document.getElementById(marqueeId)
})
  
return (
  <div id={marqueeId} />
)

2khan avatar Mar 25 '24 05:03 2khan