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

Add path.getPrevSibling, path.getNextSibling, etc. #5223

Closed
7 tasks
kangax opened this issue Jan 26, 2017 · 7 comments · Fixed by #5230
Closed
7 tasks

Add path.getPrevSibling, path.getNextSibling, etc. #5223

kangax opened this issue Jan 26, 2017 · 7 comments · Fixed by #5230
Labels
good first issue Has PR help wanted i: enhancement outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Comments

@kangax
Copy link
Member

kangax commented Jan 26, 2017

We have quite a few cases of path.parentPath.getSibling(path.parentPath.key + 1) in Babili's DCE.

It would be nice if Babel provided basic traversal methods like getPrevSibling(), getNextSibling(), forEachPrevSibling(), forEachNextSibling(), etc.

Naming subject to change, of course.


EDIT by @hzoo

  • Comment below you are going to do this (for others to know)
  • Read CONTRIBUTING.md
  • This can be done on master
  • remember to run make build after a source code change or better just run make watch in another terminal window to auto run babel while you are developing
  • Check out the handbook if you need more context into what a "sibling" is. Basically it's like the same idea with a DOM sibling ($("a").siblings) but instead its an AST node sibling.

@babel-bot
Copy link
Collaborator

Hey @kangax! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

@hzoo
Copy link
Member

hzoo commented Jan 26, 2017

I like it! Names seem fine to me but maybe someone else has a better idea. We should just have something like jQuery haha https://api.jquery.com/category/traversing/tree-traversal/.

@kangax
Copy link
Member Author

kangax commented Jan 26, 2017

Seems like a nice beginner-friendly task so I'm marking it as such.

@hzoo
Copy link
Member

hzoo commented Jan 26, 2017

Yep! I'l update the issue for that stuff.

If someone wants to chime in on a good name though..

@chitchu
Copy link
Contributor

chitchu commented Jan 27, 2017

Hi @kangax, @hzoo

If you don't mind I'll pick this one up.

I do have a suggestion. Instead of getNextSibling(), getPrevSibling(), would method chaining be better syntax wise? ie getSibling().next(), getSibling().prev(), getSibling().prev().prev().next()

@kangax
Copy link
Member Author

kangax commented Jan 27, 2017

@chitchu chaining would work automatically since the method will be defined on a path and will return another path (or null/undefined).

@chitchu
Copy link
Contributor

chitchu commented Jan 27, 2017

@kangax I misunderstood what getSibling does. That makes it clear then.

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label May 5, 2018
@lock lock bot locked as resolved and limited conversation to collaborators May 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Has PR help wanted i: enhancement outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants