repl: fix getters triggering side effects during completion #61043
Conversation
|
By the way, thanks a lot for the comment there @BridgeAR! 🫶 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #61043 +/- ##
=======================================
Coverage 88.53% 88.53%
=======================================
Files 703 703
Lines 208546 208537 -9
Branches 40217 40222 +5
=======================================
- Hits 184634 184631 -3
+ Misses 15926 15910 -16
- Partials 7986 7996 +10
🚀 New features to boost your workflow:
|
Co-authored-by: Ruben Bridgewater <ruben@bridgewater.de>
|
@dario-piotrowicz please do not repeatedly add the
request-ci
|
ok sorry about that 🙇 It seems like I've lost permissions to view the Jenkins results or manually retrigger the Jenkins jobs (thus why I just re-added the label), was the change intentional? or could I get perms back? |
|
Landed in aad8c05 |
PR-URL: nodejs#61043 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Aviv Keller <me@aviv.sh>
The changes in #59044 are incorrect, as pointed out by @BridgeAR here: #59044 (review)
Sorry somehow I totally missed that while reviewing the PR, 59044 added a try-catch around the check to see if an object property is a proxy, the reasoning being that the property could have a getter that could throw an error.
The correct solution is not to try-catch the property access, but to avoid it in the first place. We actually have already the logic to check if a property has a getter, so the correct solution consists simply in performing the proxy check after we've made sure that the property doesn't have a getter.