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

ESNext+[[Define]]: reference to param props illegal #36425

Merged
merged 2 commits into from
Jan 24, 2020

Conversation

sandersn
Copy link
Member

When target: "esnext" and useDefineForClassFields: true, property
declaration initialisers should not be able to reference parameter
properties; class fields are initialised before the constructor runs,
but parameter properties are not:

class C {
  foo = this.bar
  constructor(public bar: string) { }
}

emits code that looks like this:

class C {
  bar
  foo = this.bar
  constructor(bar) {
    this.bar = bar
  }
}
new C('x').foo.length // crashes; foo is undefined

This PR adds an error on foo's declaration with ESNext+[[Define]].

Fixes #36410

When target: "esnext" and useDefineForClassFields: true, property
declaration initialisers should not be able to reference parameter
properties; class fields are initialised before the constructor runs,
but parameter properties are not:

```ts
class C {
  foo = this.bar
  constructor(public bar: string) { }
}
```

emits code that looks like this:

```js
class C {
  bar
  foo = this.bar
  constructor(bar) {
    this.bar = bar
  }
}
new C('x').foo.length // crashes; foo is undefined
```

This PR adds an error on foo's declaration with ESNext+[[Define]].
class C {
qux = this.bar // should error
bar = this.foo // should error
quiz = this.bar // ok
Copy link
Member Author

Choose a reason for hiding this comment

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

Given the code change, I don't understand why the lines with // ok don't error. It is nice that they don't though.

@sandersn sandersn merged commit 368db99 into master Jan 24, 2020
Copy link
Contributor

@ajafff ajafff left a comment

Choose a reason for hiding this comment

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

Something in this PR broke checking of deferred access to (regular) properties. Will open an issue after investigating.

class C {
    bar = () => this.foo; // was no error in -dev.20200124, is an error in -dev.20200125
    foo = 1;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

useDefineForClassFields and parameter properties: missing error or bad emit
4 participants