Conversation
Otherwise looks good to me! |
jridgewell
left a comment
There was a problem hiding this comment.
@bakkot's request, too. Nearly there.
| clause.param = this.parseBindingAtom(); | ||
| this.checkLVal(clause.param, true, Object.create(null), "catch clause"); | ||
| this.expect(tt.parenR); | ||
| } |
There was a problem hiding this comment.
Let's set clause.param = null in an else case.
|
https://github.com/babel/babylon/blob/master/CONTRIBUTING.md#creating-a-new-plugin-spec-new Yeah I would:
|
…tionalCatchBinding
src/parser/statement.js
Outdated
| this.checkLVal(clause.param, true, Object.create(null), "catch clause"); | ||
| this.expect(tt.parenR); | ||
| } | ||
| clause.param = null; |
There was a problem hiding this comment.
I think you'll need to add this to else otherwise it would set it to null in all cases? And then rerun the tests fixtures
There was a problem hiding this comment.
Hmmm. This is causing a failure in one of my tests. I'll have to look at more.
| this.checkLVal(clause.param, true, Object.create(null), "catch clause"); | ||
| this.expect(tt.parenR); | ||
|
|
||
| if (this.match(tt.parenL) || !this.hasPlugin("optionalCatchBinding")) { |
There was a problem hiding this comment.
I was wondering whether the this.match check is necessary?
There was a problem hiding this comment.
Ok never mind, talked to Kara and it makes sense. This is for the case you have the plugin on, but you choose to still use the binding itself.
|
Nice work! 😄 First parser PR in the bag |
This implements the optional catch binding proposal (babel/proposals #7).