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

[BUGFIX] Make ArrayProxy Lazy #18835

Merged
merged 1 commit into from
Mar 24, 2020

Conversation

pzuraq
Copy link
Contributor

@pzuraq pzuraq commented Mar 24, 2020

This PR refactors ArrayProxy to only begin observing the proxied array's
changes the first time the proxy is actually used. This fixes a number
of bugs that have popped up recently, and also ensures that ArrayProxy
creation does not accidentally entangle with the autotracking stack. It
also simplifies the code overall, and conceptually is more in line with
the way that autotracking works.

In addition, it adds a CUSTOM_TAG_FOR to the proxy, to forward the []
and length tags from its content directly rather than attempting to
managed them itself. This keeps the system from encountering issues
related to tag updates, and from needing to re-notify when changes are
propagated.

As part of fixing this, I also discovered that the hasArrayObservers
API has been broken since ~3.11. I fixed it and added a test.

Original PR: #18770

This PR refactors ArrayProxy to only begin observing the proxied array's
changes the first time the proxy is actually used. This fixes a number
of bugs that have popped up recently, and also ensures that ArrayProxy
creation does not accidentally entangle with the autotracking stack. It
also simplifies the code overall, and conceptually is more in line with
the way that autotracking works.

In addition, it adds a CUSTOM_TAG_FOR to the proxy, to forward the `[]`
and `length` tags from its content directly rather than attempting to
managed them itself. This keeps the system from encountering issues
related to tag updates, and from needing to re-notify when changes are
propagated.

As part of fixing this, I also discovered that the `hasArrayObservers`
API has been broken since ~3.11. I fixed it and added a test.
@pzuraq pzuraq requested a review from rwjblue March 24, 2020 00:20
@rwjblue
Copy link
Member

rwjblue commented Mar 24, 2020

Can you cross link the original PR?

@@ -11,14 +11,14 @@ import {
removeArrayObserver,
replace,
getChainTagsForKey,
tagForProperty,
CUSTOM_TAG_FOR,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When did this API land? I just want to make sure it existed in 3.16...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it landed with one of the more recent bugfixes, and I believe was backported.

@rwjblue rwjblue changed the title [BUGFIX lts] Make ArrayProxy Lazy [BUGFIX] Make ArrayProxy Lazy Mar 24, 2020
@rwjblue rwjblue merged commit 2133e22 into lts-3-16 Mar 24, 2020
@rwjblue rwjblue deleted the bugfix-lts/fix-array-proxy-content-update branch March 24, 2020 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants