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

Change AstPath#match() #13692

Open
fisker opened this issue Oct 20, 2022 · 3 comments
Open

Change AstPath#match() #13692

fisker opened this issue Oct 20, 2022 · 3 comments
Labels
type:refactor Issues about tackling technical debt

Comments

@fisker
Copy link
Member

fisker commented Oct 20, 2022

The name argument in AstPath#match() seems work different than AstPath#each()/AstPath#map(). It is the key to access child, not the key to access node.

This confusing me, I hope we can change it.

  1. Make name as the key to access node to consist with other method/getter.
  2. Add a parent argument.

Take this as example,

https://github.com/prettier/prettier/blob/next/src/language-js/print/misc.js#L26-L30

    path.match(
      undefined,
      (node, name) =>
        name === "id" && node.type === "VariableDeclarator" && node.definite
    )

If we choose 1:

path.match(
  (_, name) => name === "id",
  (node) => node.type === "VariableDeclarator" && node.definite
)

If we choose 2:

path.match(
  (node, parent) => parent.type === "VariableDeclarator" && parent.definite && parent.id === node
)

I prefer 2nd choice.

WDYT? @thorn0

@fisker fisker added the type:refactor Issues about tackling technical debt label Oct 20, 2022
@thorn0
Copy link
Member

thorn0 commented Oct 20, 2022

I don't feel strongly about 1. But 2 looks strange. The idea of match is that there is a separate predicate for each level of the AST. Why mix levels in one predicate?

@fisker
Copy link
Member Author

fisker commented Oct 21, 2022

How about call callParent inside, so we get path as argument?

@thorn0
Copy link
Member

thorn0 commented Oct 21, 2022

What for? Do you have specific cases you want to improve this way?

BTW, what I'd like to have in match is support for alternatives, but that's quite a non-trivial feature to implement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:refactor Issues about tackling technical debt
Projects
None yet
Development

No branches or pull requests

2 participants