Skip to content

Commit

Permalink
[BUGFIX] Entangle Errors.errorsFor properly
Browse files Browse the repository at this point in the history
We recently made some changes to the way arrays get consumed when
autotracking in Ember - specifically, whenever an array is return from
`Ember.get`, `@computed`, or `@tracked`, we track that array for changes
in the future. Previously, we also tracked it if it was an _EmberArray_.
This check was a bit redundant, and costly, so we removed it and now
only do it for native arrays.

The reason this is generally safe is that non-native EmberArrays usually
have a contract - you must access the state of the array via `objectAt`,
and you must update via `pushObject`, `popObject`, etc. The array is
consumed when using `objectAt`, so theres no need to do it eagerly in
`Ember.get` (and penalize all other object access).

The `Errors` class is a bit of a special case, because it adds additonal
capabilities - the `errorsFor` method, which returns a subset of errors
in the array. This info is accessed via the `errorsFor()` method, which
does not consume the array proxy at all. So, previously, if a user did
something like:

```js
model.errors.errorsFor(field).map(error => error.message);
```

When `model.errors` was accessed, it was a `@computed` property, and it
consumed the entire array proxy. Later on, when the user goes to add an
error, it adds the error to the main array _and_ the subarray returned
by `errorsFor()`, and dirties both, so the user's code is called again
and updates properly.

With the recent changes, that doesn't happen anymore. Since `errorsFor`
returns a native array (with prototype extensions enabled), `map` does
not use `objectAt` internally either. So, there is nothing that is
properly consuming the array.

This PR adds the consumption manually. In the future, we can likely add
a primitive that allows Errors to create and manually manage tags, or it
can use something along the lines of a TrackedArray for these errors
instead.
  • Loading branch information
Chris Garrett committed Aug 19, 2020
1 parent b2e4ba1 commit f985d5e
Showing 1 changed file with 13 additions and 3 deletions.
16 changes: 13 additions & 3 deletions packages/model/addon/-private/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,21 @@ export default ArrayProxy.extend(DeprecatedEvented, {
errorsFor(attribute) {
let map = get(this, 'errorsByAttributeName');

if (!map.has(attribute)) {
map.set(attribute, A());

let errors = map.get(attribute);

if (errors === undefined) {
errors = A();
map.set(attribute, errors);
}

return map.get(attribute);
// Errors may be a native array with extensions turned on. Since we access
// the array via a method, and not a computed or using `Ember.get`, it does
// not entangle properly with autotracking, so we entangle manually by
// getting the `[]` property.
get(errors, '[]');

return errors;
},

/**
Expand Down

0 comments on commit f985d5e

Please sign in to comment.