Skip to content
This repository has been archived by the owner on Sep 27, 2020. It is now read-only.

Allow accessing node parents in visitor function #51

Open
yxliang01 opened this issue Mar 2, 2019 · 12 comments
Open

Allow accessing node parents in visitor function #51

yxliang01 opened this issue Mar 2, 2019 · 12 comments

Comments

@yxliang01
Copy link
Contributor

yxliang01 commented Mar 2, 2019

Since the AST format of this library is customized, I feel exporting the function _isASTNode is generally favored as it guarantees that when the format of output of this library changes, this check will/should always be up-to-date.

@federicobond do you agree?

@federicobond
Copy link
Owner

Hi @yxliang01, can you describe some use cases for this function? I'm not sure I understand well how it would be used.

@yxliang01
Copy link
Contributor Author

yxliang01 commented Mar 6, 2019

@federicobond Like if you want to write a custom traverser of the AST tree, this function can provide information like whether an attribute of a node is yet another AST node (This is actually my case). While the current definition of this function is very simple, it's really useful :) and I feel it's a good idea to export this to provide some encapsulations.

@federicobond
Copy link
Owner

I understand your concern. However, I would suggest to just check if the node is an object and has a type key for now, as that should be considered part of the stable interface of this parser. If any other issues come up related to this, I am willing to reconsider that decision.

@federicobond
Copy link
Owner

I think we can take this opportunity to introduce a better traverser. Please let me know what constraints are driving your custom traverser implementation, as we might be able to do something on that front.

@yxliang01
Copy link
Contributor Author

@federicobond I basically need the object path of each AST node in the AST tree. So that, I can directly visit the previous node visited node by the object path. I deal with it by adapting the visit myself, so that this information is tracked and passed to the visitor. I think this usage might be specific though.

@federicobond
Copy link
Owner

federicobond commented Mar 7, 2019

That is a pretty common usage. The format of the AST is esprima-compatible, so preprocessing it with something like ast-parents should work well too.

@yxliang01
Copy link
Contributor Author

I see. Thanks for pointing this. But, this is quite inefficient given that need to traverse the AST for once more and increases memory space by quite much I feel.
So, do you mean that it's a good idea to pass the object path information to visitors by commenting it's a common usage? Or?

@yxliang01
Copy link
Contributor Author

@federicobond any suggestion for what should be done for this issue? Otherwise, this will be stale :(

@federicobond
Copy link
Owner

Hey! Sorry, my last few days have been pretty hectic. I think the lib could implement this functionality by passing a second argument to each visit call, with the appropiate parent chain. Would be happy to review a pull request to that end.

As for checking whether something is an AST node, I think you can safely rely on a check for an object with a type property.

@federicobond federicobond changed the title export _isASTNode Allow accessing node parents in visitor function Mar 12, 2019
@yxliang01
Copy link
Contributor Author

@federicobond I am also happy to contribute to that :) . Will do it bit latter after I have done my current important tasks.

@yxliang01
Copy link
Contributor Author

To be sure, @federicobond did you mean passing the parent node object to the visitor? Or, the object path for the whole AST(my need)?

@federicobond
Copy link
Owner

Yes, passing the full object path makes a lot of sense.

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

No branches or pull requests

2 participants