New issue
Advanced search Search tips

Issue 867922 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Jul 26
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

JS autocomplete can evaluate getters, causing unintended side effects

Reported by kelly...@tcd.ie, Jul 26

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.68 Safari/537.36

Steps to reproduce the problem:
1. Define a class with a getter
2. Create instance of class
3. Access instance.getter. 
4. As soon as the . is entered after getter, the getter is called

Example: 

class SideEffectTest {
  constructor() {
    this.count = 0;
  }

  get c() {
    console.log(this.count);
    console.trace();
    this.count++
    return this.count;
  }
}

let test = new SideEffectTest()

console.assert(test.count == 0)
console.assert(test.c == 1)

// Begin typing test.c. then clear

// test.count has been incremented

console.assert(test.c == 2)
// Assertion Fails

What is the expected behavior?
There would be no side effects

What went wrong?
test.count is incremented

Did this work before? N/A 

Chrome version: 68.0.3440.68  Channel: beta
OS Version: OS X 10.12.6
Flash Version: 

Low priority in the grand scheme of things, just an interesting quirk found while testing if eager evaluation can in fact cause side effects
 
Cc: kozy@chromium.org
Components: -Platform>DevTools Platform>DevTools>JavaScript
Owner: l...@chromium.org
Status: Assigned (was: Unconfirmed)
Cc: einbinder@chromium.org
Status: Archived (was: Assigned)
Summary: JS autocomplete can evaluate getters, causing unintended side effects (was: Eager evaluation can cause unintended side effects)
Thanks for the report and detailed case.  EagerEval is intended to be side-effect-free, but I'm able to reproduce, even with Eager Eval disabled.  This behavior is due to our autocomplete logic, which aggressively evaluates getters in some cases.

With a similar case, I can repro as far back as Chrome 49.0.2623.0:
`
var obj = {count: 0};
Object.defineProperty(obj, 'c', {get: function() {console.log('foo')}})
// type 'obj.c.'
`

Today, our side-effect checker relies on marking common attributes as 'safe'.  For example, less-common attributes like `window.history.scrollRestoration.` still have autocomplete, but do not have an EagerEval preview.

Making autocomplete side-effect-free today would break those less-common cases, and I'm not sure it is worth it just yet.  When we have more resources to make the side-effect checker more robust, I think we could make the switch and avoid calling getters.  Unless einbinder@ or kozy@ have other ideas, I'd like to Archive this for now.

Please let us know if you have questions or ideas on how we could convey this better :)

Sign in to add a comment