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

Feat: Some type of way to merge key prop and useOpaqueIdentifier result #20822

Closed
SoAsEr opened this issue Feb 14, 2021 · 3 comments
Closed

Feat: Some type of way to merge key prop and useOpaqueIdentifier result #20822

SoAsEr opened this issue Feb 14, 2021 · 3 comments
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug Type: Enhancement

Comments

@SoAsEr
Copy link

SoAsEr commented Feb 14, 2021

Currently, the expected way to handle an unknown at compile time number of labels is to put them each in their own component. However, when creating tables, this causes a mess of callbacks, as the only way to do it (that I can think of anyway) is to pass a callback into the table header components, have them create their own unique identifier, and then use a useEffect to give the parent those identifiers, have the parent pass those identifiers to the rows, and then have each row generate its opaque own unique identifiers and pass both to the cells. Note that this doesn't work currently because of these two bugs:
#18594
#20127

However, react already has a way to differentiate items in loops: the key prop. To me, it would make sense to provide a function which merges a key prop with a unique identifier. This would allow the parent to generate the unique identifiers and pass them to the children, rather than having callbacks which cause a rerender. It would even make #20127 less relevant because there would be fewer motivating reasons to do so.

mergeOpaqueIdentifierAndKey is very wordy, but describes exactly what's going on. I don't have any particular preference about the name.

@SoAsEr SoAsEr added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Feb 14, 2021
@eps1lon
Copy link
Collaborator

eps1lon commented Feb 14, 2021

Prior art reactjs/rfcs#32 (comment).

Naively we could just append the key to the generated id. But this would increase the number of bytes sent when doing SSR and also put new constraints on what kind of strings we can pass to key

@SoAsEr
Copy link
Author

SoAsEr commented Feb 19, 2021

The other option would be to sort the keys added once we start the toString and then append their index. We wouldn't even have to use string compare at first. We could sort by length (which is faster) and then fall back to string comparison if their lengths are equal.

SoAsEr added a commit to SoAsEr/react that referenced this issue Feb 26, 2021
@SoAsEr
Copy link
Author

SoAsEr commented Nov 11, 2021

New approach is totally different and doesn't need this complicated scheme: reactwg/react-18#111

@SoAsEr SoAsEr closed this as completed Nov 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug Type: Enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants