Skip to content

removing undefined from AstPath<PrintableNode | undefined>#1512

Merged
Janther merged 1 commit intomainfrom
get-visitor-keys
Apr 28, 2026
Merged

removing undefined from AstPath<PrintableNode | undefined>#1512
Janther merged 1 commit intomainfrom
get-visitor-keys

Conversation

@Janther
Copy link
Copy Markdown
Member

@Janther Janther commented Apr 13, 2026

since it's not required with our current print strategy.

as a bonus I added getVisitorKeys to minimise the visits to nodes prettier will make during their process. (there is a small bump in the speed of our tests)

Also @fvictorio what's your input on prettier/prettier#19041 (comment)? I'm inclined in keeping our printing approach as is and only address it if in a future version, prettier decides to do something different when passing undefined like throwing.

@Janther Janther requested a review from fvictorio April 13, 2026 02:33
@Janther Janther force-pushed the get-visitor-keys branch 2 times, most recently from dc09696 to 4ba5e73 Compare April 17, 2026 15:58
@Janther Janther force-pushed the get-visitor-keys branch 2 times, most recently from ecb0bdc to b815f83 Compare April 25, 2026 16:38
@fvictorio
Copy link
Copy Markdown
Member

Also @fvictorio what's your input on prettier/prettier#19041 (comment)? I'm inclined in keeping our printing approach as is and only address it if in a future version, prettier decides to do something different when passing undefined like throwing.

I'm not sure I fully understand the situation, but it seems to boil down to:

  1. There's an API that receives T.
  2. Internally, the API also handles T | undefined (and also null I guess, it doesn't matter).
  3. Since the type doesn't say | undefined, the implementation could stop handling undefined values (e.g., throw).

So our options are to keep relying on that implicit handling or to handle it ourselves.

Is that more or less correct?

If so: I wouldn't do anything. For this to be a problem, several things have to happen (prettier core makes what amounts to a breaking change, our own quite strict typescript usage passes an undefined, etc.), and if it does happen it's probably going to be loud and easy to figure out and handle.

@Janther
Copy link
Copy Markdown
Member Author

Janther commented Apr 28, 2026

yeah yeah, I agree, one good thing came out of this. I had to rethink how much we depended on this and noticed #1518 was a good thing to change.

@Janther Janther merged commit a916a26 into main Apr 28, 2026
7 checks passed
@Janther Janther deleted the get-visitor-keys branch April 28, 2026 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants