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

Symbols as keys in children as arrays or iterators #11996

Closed
vectorsize opened this issue Jan 9, 2018 · 18 comments
Closed

Symbols as keys in children as arrays or iterators #11996

vectorsize opened this issue Jan 9, 2018 · 18 comments

Comments

@vectorsize
Copy link

Do you want to request a feature or report a bug?

I want to request a feature

What is the current behavior?

Using Symbols as element keys throws a type error.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:

When using key={Symbol('myKeySymbol')} we get the following TypeError: Cannot convert a Symbol value to a string at Object.ReactElement.createElement

codesandbox here

What is the expected behavior?
Using Symbols as keys should work seamlessly, in my opinion element keys are a perfect use-case for Symbols.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

Using React 16.0.0 and the browsers affected are Safari, Chrome and Firefox on OSX, but I'm pretty sure this is not browser dependent but a matter of implementation.


Thanks.
@gaearon
Copy link
Collaborator

gaearon commented Aug 9, 2018

Can you explain the use case? Why are Symbols useful as keys?

@gaearon gaearon changed the title Symbols as keys in children as arrays or itterators Symbols as keys in children as arrays or iterators Aug 9, 2018
@vectorsize
Copy link
Author

Well it's kind of simple really, even naïve perhaps. Since keys need to be unique and for the application logic kind of purposeless I thought it would be nice using symbols that are always unique without even have to worry about coming up with unique names.

@gaearon
Copy link
Collaborator

gaearon commented Aug 9, 2018

I think that's a misunderstanding of the purpose of the keys.

Keys don't need to be unique globally. They need to be unique among siblings. They don't need to be unique just to avoid clashes or something like this — they need to represent a stable identity of some data you're iterating over. So typically they would come from your data: such as a customer ID, or username.

See these explanations:

https://reactjs.org/docs/lists-and-keys.html#keys
https://reactjs.org/docs/glossary.html#keys
https://reactjs.org/docs/reconciliation.html#keys

You don’t need to “come up with unique names” — they should already be available in your data. Every time you iterate a list of something (users, rows, items, products, comments, posts), you should be able to use IDs of those items as keys.

So I don’t see a practical use case for allowing Symbols, except a misunderstanding.

@gaearon gaearon closed this as completed Aug 9, 2018
@aweary
Copy link
Contributor

aweary commented Aug 9, 2018

Another potential problem with using Symbols is that it might obscure issues with unintentional data duplication. Since two symbols are never equal, even if they have the same value, you won't get a useful key warning about duplicate items---even if all the Symbol inputs were identical.

<ul>
   {/* Duplicate items, but React cant warn you about it */}
  <li key={Symbol('1')}>1</li>
  <li key={Symbol('1')>1</li>

This would also be problematic if users defined Symbols as keys inline

<div key={Symbol('foo')} />

key would be different on every render, which would force React to re-mount the element/component. You'd have to always use Symbol.for to avoid these problems, which would be an easy pitfall for most users.

@dakom
Copy link

dakom commented Mar 27, 2019

Counterpoint: a symbol may already exist on the data and it makes sense to re-purpose it in that case, no?

@smhg
Copy link

smhg commented May 16, 2019

Isn't a Symbol a perfect match for a temporary id (because unique) while you're awaiting receiving one from the server?

// ...
handleAdd (item) {
  const collection = new Map(this.state.collection);
  const tmpId = Symbol('mycollection');

  collection.set(tmpId, item);

  this.setState({ collection });

  saveItem(item)
    .then(item => {
      const collection = new Map(this.state.collection);

      collection.delete(tmpId);

      collection.set(item.id, item);

      this.setState({ collection });
    });
}
// ...

Currenlty you have to convert the Symbol to a string yourself like this:

<div key={String(id)}>{...}</div>

Not a big hassle of course, but I'd be grateful if @gaearon could validate/invalidate this usage.

@esanzgar
Copy link

esanzgar commented Sep 11, 2019

In my opinion, supporting Symbol as key could be useful in this situation:

import React from 'react';

const TODO = [
  {id: Symbol(), content: 'Wake up'},
  {id: Symbol(), content: 'Go to sleep'},
];

const App = () => {
  return (
    <>
      {TODO.map(({id, content}) => (
        <p key={id}>{content}</p>
      ))}
    </>
  );
};

export default App;

So there is no need to rely on Date or uuid libraries.

@gopisanto
Copy link

Using Symbols is highly discouraged by me, because imagine there are 3 keys with "one", "two" and "three", assume you are changing or modifying data related to key "three", then if our code is good then it should not touch UI with keys "one" and "two", but in this case it will re-render them as well because, when you build the key using Symbol it will not be same next time even when the values are not changed. hence i will strongly discourage using Symbols. Let me know if some one agrees to me or correct me if something is wrongly perceived by me.

@jzarzeckis
Copy link

@gopisanto Do I imagine your example correctly?

const state = [
  { name: 'one', key: Symbol() },
  { name: 'two', key: Symbol() },
  { name: 'three', key: Symbol() }
]

const updatedState = state.map((item, i) =>
  i === 2 ? {...item, name: 'modified three'} : item
)

Here the 3rd item has its name changed, but the key remains untouched for all items.

@gopisanto
Copy link

This is the only object you have defined, but can you please tell me how you are iterating over this object while building some jsx and then how you are giving the key to jsx.

@angry-dan
Copy link

angry-dan commented Jan 28, 2020

Edit: ignore this - I just realised my own mistake that Symbol.for doesn't care about object identity, so Symbol.for('foo') isn't unique enough anyway.

Say I want to make the simplest todo list (no storage, items stored in state only)
I have a state object holding an array of todos (strings): ['buy milk', 'book appointment']

Now I want to render that list, I could use the string as a key, but my list is user generated content, and for some reason I might well create the same todo twice. So I could use key={Symbol.for(str)} and as long as I follow immutable principles the key would be stable for re-ordering the list and deleting items.

@ElijahKaftanov
Copy link

@gaearon @bvaughn
ok, here is an issue
so I have a list of messages in a chat. And I want to add a new message to this list before the message even saved on my server. I don't have an id from db for this message, so my unique key could not be the id field. Thus I need to create a new unique key for each message. Creating and, more importantly, comprising strings is too slow I am starting to think about Symbols

@bvaughn
Copy link
Contributor

bvaughn commented Nov 6, 2020

You're commenting on the closed issue which is not as likely to get feedback.

Creating and, more importantly, comprising strings is too slow I am starting to think about Symbols

I don't really know what this means.

@ElijahKaftanov
Copy link

ElijahKaftanov commented Nov 6, 2020

@bvaughn
it's just a continuation of the talk. Not a real issue, so I decided not to open a new one.

Let's say I have a thousand messages on a list. To find the exact message I need to traverse through all items and comprise the keys. Weren't symbols created for these certain tasks?

example thunk:

const spawnSendMessage = (message: Message) => async (d, g) => {
  const uid = Symbol.for('chat.message.uid');

  d(spawnPushMessage({uid, sent: false, ...message}));

  const id = await gate.apx({
    action: 'chat.message.send',
    body: message
  });

  const messages = findAndUpdate(
    msg => msg.uid === uid,
    msg => ({...msg, id, sent: true}),
    selectMessages(g())
  )
  
  d(spawnSetMessages(messages));
}

@bvaughn
Copy link
Contributor

bvaughn commented Nov 6, 2020

it's just a continuation of the talk. Not a real issue, so I decided not to open a new one.

My suggestion was not to open a new one, but to comment on the issue that was already open: #19851

Although I'd suggest reading the discussion on that issue first– as using a Symbol for a key has some drawbacks too.

@Findiglay
Copy link

I think a temporary list of elements is a valid use case here. Symbol would work well for this since it is guaranteed to be unique.

@pauldraper
Copy link

pauldraper commented Dec 29, 2023

Angular allows any value to be used as the key.

If you want something like that,

let key = 0;

const keys = new WeakMap<any, number>();

export function getKey(value: any): string | number {
  switch (typeof value) {
    case "bigint":
      return `bigint:${value}`;
    case "number":
      return value;
    case "string":
      return `string:${value}`;
    case "object":
      if (value === null) {
        return "null";
      }
    case "symbol":
      if (keys.has(value)) {
        return keys.get(value)!;
      }
      const k = key++;
      keys.set(value, k);
      return k;
    case "undefined":
      return "undefined";
  }
  throw new Error(`Unsupported: ${value}`);
}
[Symbol(), Symbol()].map(x => <div key={getKey(x)} />)

@tomtheisen
Copy link

@pauldraper This is a pretty elegant workaround, but be aware that as of Firefox 123, this is broken for symbols:

(new WeakMap).set(Symbol(),1) 
Uncaught TypeError: WeakMap key Symbol() must be an object or an unregistered symbol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests