Skip to content

Using node instead of getNode() - #1167

Merged
Janther merged 20 commits into
mainfrom
node-getter
Jul 9, 2025
Merged

Using node instead of getNode()#1167
Janther merged 20 commits into
mainfrom
node-getter

Conversation

@Janther

@Janther Janther commented Jun 11, 2025

Copy link
Copy Markdown
Member

it has a cleaner type for the type checks, and the standard in prettier's parsers.
using the same approach, getNode(2) was replaced by grandparent.
I also added some really quick refactors that I found when replacing these cases and don't really merit another PR.

@Janther
Janther requested a review from fvictorio June 11, 2025 21:03
@Janther
Janther force-pushed the node-getter branch 7 times, most recently from 5c85813 to e006407 Compare June 24, 2025 08:34
@Janther
Janther force-pushed the node-getter branch 4 times, most recently from b8e6c72 to 0ed5a67 Compare June 27, 2025 10:38
@Janther Janther added refactor reorganising the code without changes performance Results in smaller size, better memory usage, or faster execution labels Jun 28, 2025
@Janther
Janther force-pushed the node-getter branch 3 times, most recently from 7e0fe1a to c78ce5c Compare July 7, 2025 19:11
@Janther
Janther force-pushed the node-getter branch 2 times, most recently from b3cbaa4 to 78c375c Compare July 8, 2025 08:33

@fvictorio fvictorio left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't agree with some of the refactors here (e.g., I think destructuring + aliases like const { foo: bar } = qux are generally less readable than const bar = qux.foo), but since this is mostly a personal preference, I'm ok with merging this PR as is.

@Janther
Janther merged commit 530d187 into main Jul 9, 2025
7 checks passed
@Janther
Janther deleted the node-getter branch July 9, 2025 17:19
@Janther

Janther commented Jul 9, 2025

Copy link
Copy Markdown
Member Author

happy to review these destructures in most cases I use the alias in a function signature where I don't have access to the = and renaming everything to the property name would look weird:

function printComment({ node: comment }) {
  if (isComment(comment)) {
    return comment.print();
  }

  throw new Error(`Not a comment: ${JSON.stringify(comment)}`);
}

And the other place where I use the alias is in a for...of loops with a similar reason.

    for (const { variant: attribute } of this.attributes.items) {
      if (
        typeof attribute !== 'string' &&
        attribute.kind === NonterminalKind.ModifierInvocation
      ) {
        attribute.cleanModifierInvocationArguments();
      }
    }

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

Labels

performance Results in smaller size, better memory usage, or faster execution refactor reorganising the code without changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants