Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport: 3.16.9 lts #7291

Merged
merged 6 commits into from
Aug 29, 2020
Merged

Backport: 3.16.9 lts #7291

merged 6 commits into from
Aug 29, 2020

Conversation

snewcomer
Copy link
Contributor

No description provided.

Chris Garrett and others added 3 commits August 28, 2020 14:38
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.
This hasn't worked for some period of time (Ember itself forbids
`new EmberObject()`), removing the test for `new` since it is tested
upstream by Ember.
`willMergeMixin` was private API (and was removed in
[email protected]), this moves the warning into `init` (provides the
same functionality) instead.
@snewcomer snewcomer added the backport-lts PR targets the current lts branch label Aug 28, 2020
@snewcomer snewcomer self-assigned this Aug 28, 2020
@snewcomer snewcomer merged commit 0e8fb56 into lts-3-16 Aug 29, 2020
@delete-merged-branch delete-merged-branch bot deleted the backport/3.16.9 branch August 29, 2020 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-lts PR targets the current lts branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants