firebase / flutterfire

🔥 A collection of Firebase plugins for Flutter apps.
https://firebase.google.com/docs/flutter/setup
BSD 3-Clause "New" or "Revised" License
8.67k stars 3.97k forks source link

[cloud_firestore] DocumentSnapshot data should be immutable #2007

Closed collinjackson closed 3 years ago

collinjackson commented 4 years ago

In the past it was possible to mutate the data member of a DocumentSnapshot and it would persist. Developers were relying on this behavior (see discussion here).

We could make the data returned by DocumentSnapshot immutable to give a clear error to developers letting them know that mutations aren't supported instead of silently failing. This would also reduce the risk of unpredictable behavior when different parts of the code are mutating the same data map.

For bonus points we could lazily convert the maps and cache the result to avoid unnecessary computation.

ditman commented 4 years ago

@collinjackson is this a regression (since users were able to do this in the past) or an enhancement (since we want to be more proactive against this in the future)?

collinjackson commented 4 years ago

I think developers were relying on an undocumented behavior... it's mostly an enhancement request to be more proactive against this, unless someone wants to convince me that this is really a good idea.

I've updated the label.

Let me know if you feel otherwise!

ditman commented 4 years ago

Nope, I wanted to change the label to enhancement as well, but I wasn't sure if the old behavior was desired or not.