1494318 - Top-level await creates global properties

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P1)

Description

[Oriol Brufau [:Oriol]]

Run this code:

  let foo = 123;
  Object.getOwnPropertyDescriptor(window, 'foo');

It produces `undefined`. Then run this one:

  let bar = await 123;
  Object.getOwnPropertyDescriptor(window, 'bar');

It produces a property descriptor object.

It can also trigger setters:

  Object.defineProperty(window, "baz", {set(v) { console.log("Set", v) }});
  let baz = await 123; 

I haven't read the top-level await proposal for ECMAScript, but don't think 'let' declarations are supposed to behave like this.

Comment 1

[Nicolas Chevobbe [:nchevobbe][PTO - back on October 21]]

Jason, I guess this is because of the mapping from `let x = ` to `self.x = ` ?

Comment 2

[Jason Laster [:jlast]]

yep. although, this shouldnt happen unless we're paused.

We might want to change this so shouldMapBindings is false unless we're paused
https://github.com/nchevobbe/debugger.html/blob/mapexpression/src/workers/parser/mapExpression.js#L40

Comment 3

[Nicolas Chevobbe [:nchevobbe][PTO - back on October 21]]

So, this is not only related to top-level await, but basically each time we go through the parser worker to modify the input.
This happens when the debugger is open with the split console for instance. 
The following would result in a warning message:  

```js
Object.defineProperty(window, "baz", {set(v) { console.warn("That's a trap", v) }});
let baz = await 123;
```

Like Jason said, we should only do that when we are paused, which seems simple to do.
**But**, that leads us to an issue with top-level await expression.

Let's say we have this `let x = await 42`.
Under the hood, it gets transpiled into :
```js
(async () => {
  return self.x = await 42;
})()
```

Since, we don't want the `self` bit when not paused, we need to transform it into:

```js
let x;
(async () => {
  return x = await 42;
})()
```
i.e., we need to go through every assignment and define it outside of the async iife.
That means we also need to handle things like `let [a,b] = await [1,2]` and `let {a,b} = await ({a: 1, b: 2})`.

Comment 4

[Nicolas Chevobbe [:nchevobbe][PTO - back on October 21]]

Attachment: Bug 1494318 - Don't map expression bindings when debugger is not paused; r=bgrins.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7f8d4ce4ca3a
Don't map expression bindings when debugger is not paused; r=bgrins.

Comment 6

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/7f8d4ce4ca3a