This repository has been archived by the owner on Feb 12, 2022. It is now read-only.
Abstract empty values should fail RequireObjectCoercible #2515
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.
This fixes a bug it took me a while to track down. Unfortunately creating a repro was really difficult. I was observing the following invariant in
Reference
being triggered.https://github.com/facebook/prepack/blob/fddfc6f0c52fa8f49fb0cb1208f34cefcb95dca3/src/environment.js#L1425-L1431
Digging in, the bad base was a deep conditional
AbstractValue
where all the leaf nodes wereEmptyValue
and__bottomValue
. (Constructed from a number oftry
/catch
/throw
s.) ThisAbstractValue
should have simplified to justEmptyValue
thenRequireObjectCoercible()
should have thrown a runtimeTypeError
after seeing theEmptyValue
. This is what I observed in local tests when trying to reproduce this issue.After looking at the error in the large React Native bundle I’m working in I discovered that the simplifier was throwing an error since the simplification count was exceeded. This is fine and normal. The issue was that
RequireObjectCoercible()
does not handle complex emptyAbstractValue
s well. If failing simplification is a normal part of Prepack operation we should make sure methods likeRequireObjectCoercible()
can handle complexAbstractValue
s as well as simple ones.This PR does that.