Allow named properties on reactive arrays. #5216
Conversation
Without the cast to any, flow complains that Array doesn't have an __ob__ property. This appears to be an instance of this issue: facebook/flow#1330
| target.splice(key, 1) | ||
| return | ||
| } | ||
| const ob = target.__ob__ | ||
| const ob = (target : any).__ob__ |
There was a problem hiding this comment.
I can't understand this change. Could you explain?
There was a problem hiding this comment.
I can't say I understand it either, but without this cast Flow complains that Array doesn't have an __ob__ property -- why it decided that target is an array, I don't know. It appears to be an instance of this Flow issue, though my knowledge of Flow is minimal so I can't tell for sure.
|
I'm not sure if this is a good idea - custom properties on Arrays are ignored during JSON.stringify and cannot be persisted. I personally would never rely on them. |
|
I'm confused -- what do reactive properties have to do with persistence? I certainly don't do this often myself, but it's occasionally useful to pass metadata through an API/SPI where you control both ends but can only transmit an array through the library. IMHO the principle that you can safely replace any member assignment with a call to |
|
@pkaminski yeah, I guess it doesn't matter that much. I'm just thinking about the implications this has to do with normal persistence scenarios and in particular Vuex snapshots. |
|
Honestly, I don't even care if the resulting named array property is reactive (though I think it should be for consistency), I'd just like |
|
Thanks! |
* Allow named properties on reactive arrays. * Remove semicolons to comport with style guide. * Pacify flow type checking. Without the cast to any, flow complains that Array doesn't have an __ob__ property. This appears to be an instance of this issue: facebook/flow#1330
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing applications:
The PR fulfills these requirements:
devbranch for v2.x (or to a previous version branch), not themasterbranchfix #xxx[,#xxx], where "xxx" is the issue number)If adding a new feature, the PR's description includes:
Other information:
Since arrays can be treated as normal objects in JavaScript and contain a mix of numeric and named properties, this patch makes
Vue.setandVue.deldeal correctly with named properties on reactive arrays.