Skip to content
This repository has been archived by the owner on Feb 6, 2023. It is now read-only.

Why is createEntity mutable? #1256

Open
danielkcz opened this issue Jun 21, 2017 · 2 comments
Open

Why is createEntity mutable? #1256

danielkcz opened this issue Jun 21, 2017 · 2 comments

Comments

@danielkcz
Copy link

danielkcz commented Jun 21, 2017

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

What is the current behavior?
Calling contentState.createEntity actually modifies contentState object instead of just returning a new one. Here is a simple fiddle: https://jsfiddle.net/FredyCr/32vo35tk/2/

What is the expected behavior?
I guess it's not a big deal, it's just little bit confusing given that almost every other operation is immutable.

Which versions of Draft.js, and which browser / OS are affected by this issue? Did this work in previous versions of Draft.js?
Working with the latest 0.10.1, not sure about previous versions.

@flarnie
Copy link
Contributor

flarnie commented Jul 7, 2017

That is confusing - in Draft v0.11.0 it will be immutable, and here is the reason it is quirky right now:

  • Originally the 'Entity' creation/deletion API was global and mutable, separate from contentState.
  • This was inconsistent and we decided to move it to an Immutable store inside of contentState. But to temporarily support both the old and new API, for an easy upgrade, in Draft v.0.10.0 we added the new API but under the hood call the old mutable Entity API.
  • In Draft 0.11.0 we will delete the old mutable Entity API and swap in the new immutable one under the hood of the new contentState API.

For more context see #839 and https://draftjs.org/docs/v0-10-api-migration.html#content

It would be nice to clarify this in the API migration guide and docs. Would you like to take a shot at it @FredyC ?

@danielkcz
Copy link
Author

Thanks for clarification @flarnie, it makes complete sense. I guess the confusion comes mainly from examples where you can see const contentStateWithEntity = contentState.createEntity(...). Maybe avoiding that assignment to a new variable could be enough? That way it would be clear, that nothing new is returned thus mutation is expected.

What's actual release plan for 0.11.0? Looking at the branch, it seems kinda stuck there, latest change is 4 months old.

In my opinion, it's probably not worth it do an extensive update to docs. I can create PR to update that confusing piece of code in examples. What do you think?

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

No branches or pull requests

2 participants