fix(document): make array getters avoid unintentionally modifying array, defer getters until index access instead #13774
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
#13748 points out that accessing
user.hobbies
actually modifies thehobbies
array if there's a getter on elements of thehobbies
array. That is a problem. To fix that, I moved the logic for applying getters fromSchemaArray.prototype.applyGetters()
, which is also called when you accessuser.hobbies
, and into the top-levelapplyGetters()
function, which is only called bytoObject()
andtoJSON()
.As a side effect, in order to avoid breaking Schema UUID tests, we instead need to apply getters on index access. So
user.hobbies[0]
will fire getters onhobbies.0
, but not modifyhobbies
. That's what the changes totypes/array/index.js
are for.Still to investigate: it looks like
oneUser.get('hobbies.0')
double calls getters with this PR. Worth checking before merging.Also need to think about whether to merge this in 7.5, or wait for 8.0. The
types/array/index.js
changes seem a bit too heavy for a patch release.Examples