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

Don't replace missing object members with undefined #1567

Merged
merged 4 commits into from
Apr 12, 2021

Conversation

mischnic
Copy link
Contributor

@mischnic mischnic commented Apr 11, 2021

In general, {}.foo shouldn't be replaced with undefined because

  1. the properties on the object prototype aren't undefined ({}.constructor itself, {}.hasOwnProperty, ...) and
  2. hypothetically, the object's prototype could have been modified: ({}.__proto__).foo = 2; console.log({}.foo)

Instead, only member accesses for properties that actually exists should be replaced

The fix seems to work, but I'm not so sure about the added test: reverting the change to the simplifier, this is the test output:

---- simplify::branch::tests::test_object_literal stdout ----
----- Actual -----
----- Actual -----
----- Actual -----
----- Actual -----
----- Actual -----
>>>>> Orig <<<<<
({}).foo
>>>>> Code <<<<<

thread 'simplify::branch::tests::test_object_literal' panicked at 'assertion failed: `(left == right)`
            ({
}).foo;

shouldn't it say something like Expected: '({}).foo', Actual: 'undefined'?

@CLAassistant
Copy link

CLAassistant commented Apr 11, 2021

CLA assistant check
All committers have signed the CLA.

@mischnic mischnic force-pushed the object-literal-lookup branch from d0206c1 to 0bcd910 Compare April 11, 2021 13:50
@kdy1 kdy1 added this to the v1.2.53 milestone Apr 12, 2021
Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@kdy1 kdy1 enabled auto-merge (squash) April 12, 2021 09:36
@kdy1 kdy1 merged commit e43de77 into swc-project:master Apr 12, 2021
@mischnic mischnic deleted the object-literal-lookup branch April 12, 2021 15:06
@swc-project swc-project locked as resolved and limited conversation to collaborators Nov 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants