Skip to content
This repository has been archived by the owner on Aug 18, 2021. It is now read-only.

Fix parent not being set for decorator nodes. #296

Merged
merged 1 commit into from
Apr 25, 2016

Conversation

fatfisz
Copy link
Contributor

@fatfisz fatfisz commented Apr 17, 2016

I was getting strange errors when I put an arrow function into a decorator as an argument (normal function expression was affected as well).

const someObject = {
  @decorator((x) => x)
  method() {}
};

The error was coming from the no-unused-vars rule; the complaint was this: Cannot read property 'type' of undefined, at this line:

if (def.node.parent.type === "Property" && def.node.parent.kind === "set") {

So it was obvious for me that the parent property of def.node was missing.

After digging around for quite a long time (I'm unfamiliar with the inner workings of the eslint family tools...) I created this solution. Basically this is a monkey patch of estraverse that adds visitor key "decorators" to appropriate node types. That way decorators will be picked up by the enter hook of the eslint traverser, and so the parent property is set for nodes in the decorator node tree.

@hzoo
Copy link
Member

hzoo commented Apr 21, 2016

It makes sense as the babel ast is somewhat different (in the way that you modified), but it show be covered with assign(estraverse.VisitorKeys, t.VISITOR_KEYS);

@fatfisz
Copy link
Contributor Author

fatfisz commented Apr 21, 2016

So should I make a PR to babel-types then?

@hzoo
Copy link
Member

hzoo commented Apr 21, 2016

No because babel doesn't use those ast nodes anymore.

Oh wait I see we're adding decorators 😛... Ok this is correct, we might need to add more types from babel-types based on the ast changes.

@fatfisz
Copy link
Contributor Author

fatfisz commented Apr 21, 2016

So... yes or no? 😄 And if yes, then where in babel-types would it be the best to add these changes?

@hzoo
Copy link
Member

hzoo commented Apr 21, 2016

No I was just saying your PR should be the correct change

@fatfisz
Copy link
Contributor Author

fatfisz commented Apr 21, 2016

Ok, misunderstood that :)

@danez
Copy link
Member

danez commented Apr 25, 2016

Speaking of a more generic solution, we probably should copy the VisitorKeys for all nodes which were renamed in babylon to their espree name before assigning them to the different tools. Is that what you mean @hzoo?

@hzoo
Copy link
Member

hzoo commented Apr 25, 2016

Yeah

@fatfisz
Copy link
Contributor Author

fatfisz commented Apr 25, 2016

Wouldn't it be ok to patch babel-eslint now and refactor later - when there is a need for it? I'm sorry if I'm being ignorant, I just don't understand why it takes so long to ship a fix for a small bug like this :(

@hzoo
Copy link
Member

hzoo commented Apr 25, 2016

Sorry; No reason - just busy and might not be around if we need to revert but we can go ahead do this.

@hzoo hzoo merged commit cae8b66 into babel:master Apr 25, 2016
@fatfisz fatfisz deleted the fix-decorator-params-parent branch April 25, 2016 21:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants