Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add unist-util-ancestor to list of utilities #53

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

Merged
wooorm merged 2 commits into syntax-tree:main from gorango:main
Sep 23, 2021
Merged

Add unist-util-ancestor to list of utilities #53

wooorm merged 2 commits into syntax-tree:main from gorango:main
Sep 23, 2021

Conversation

@gorango
Copy link
Contributor

@gorango gorango commented Sep 22, 2021

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and couldn’t find anything (or linked relevant results below)
  • If applicable, I’ve added docs and tests

Description of changes

Adding a small utility to readme that helps with finding common ancestors of one or more unist nodes. It has come in handy a few times while working with ast trees so I thought I'd package it up. Here's hoping it might be helpful to someone else too!

P.S. I am open to renaming the package if desired.

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Sep 22, 2021
Copy link
Member

wooorm commented Sep 23, 2021

Nice!

For performance reasons you might want to not use unist-util-find (in this way). And perhaps also getParents like this.
Right now, for every node, the whole tree is walked many many times.

If possible, it would be muuuch faster to walk the tree once/twice.
Here’s some pseudocode:

const stacks = new Map()
visitParents(tree, (node, parents) => {
 if (nodesToFind.includes(node)) {
 stacks.set(node, parents)
 }
})
nodesToFind.forEach(node => {
 if (!stacks.has(node)) {
 throw new Error('unist-util-ancestor requires all nodes be contained in the tree')
 }
})
// Somethign like this?
let index = -1
let ancestor = tree
// This is broken, maybe you get the gist?
while (++index) {
 const nextAncestor = stacks.get(nodesToFind[0])[index]
 const shared = nodesToFind.every(node => stacks.get(node)[index] === nextAncestor)
 // To do: exit if there are no nodes left on stacks.
 if (shared) {
 ancestor = nextAncestor
 } else {
 break
 }
}

@wooorm wooorm added the 💪 phase/solved Post is done label Sep 23, 2021
@wooorm wooorm merged commit 3d0d9bb into syntax-tree:main Sep 23, 2021
@github-actions github-actions bot removed the 🤞 phase/open Post is being triaged manually label Sep 23, 2021

This comment has been minimized.

@wooorm wooorm added 📚 area/docs This affects documentation 🦋 type/enhancement This is great to have labels Sep 23, 2021
gorango added a commit to gorango/unist-util-ancestor that referenced this pull request Sep 23, 2021
Copy link
Contributor Author

gorango commented Sep 23, 2021

Thank you so much for taking the time to review and provide this great feedback!

I have implemented your suggestions and removed the find util completely. Way cleaner :)

Copy link
Member

wooorm commented Sep 23, 2021

That looks great! Nice find as well on the things that I didn't get to. Glad it worked!

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

Reviewers

@wooorm wooorm wooorm left review comments

Assignees

No one assigned

Labels

📚 area/docs This affects documentation 💪 phase/solved Post is done 🦋 type/enhancement This is great to have

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

AltStyle によって変換されたページ (->オリジナル) /