Skip to content

Port optional catch clause variable declaration transform #1273

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
merged 1 commit into from
Jun 24, 2025

Conversation

weswigham
Copy link
Member

This is quite possibly our simplest downlevel transform.

Perhaps obviously, this is based on #1270, so also contains it. If that's not merged yet, go look there first, before looking over this - because 3352760 is actually quite a small change.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds support for downleveling optional catch clauses by ensuring every catch without a variable gets an auto-generated identifier, and updates test baselines to expect the new explicit catch bindings.

  • Introduces optionalCatchTransformer under internal/transformers/estransforms to handle ES2019 optional catch clauses.
  • Renames and exposes various utility functions (e.g. isGeneratedIdentifierIsGeneratedIdentifier) in internal/transformers/utilities.go.
  • Updates numerous baseline files under testdata/baselines to include explicit catch variables in downlevel targets.

Reviewed Changes

Copilot reviewed 62 out of 63 changed files in this pull request and generated no comments.

File Description
internal/transformers/estransforms/optionalcatch.go Add transform for optional catch clauses
internal/transformers/utilities.go Export transformer helper functions; remove unused
internal/transformers/transformer.go Pipeline builder removed; only Factory & TransformSourceFile remain
testdata/baselines/reference/.../usingDeclarations..js Update baselines to use catch (_a) instead of catch
Comments suppressed due to low confidence (2)

internal/transformers/transformer.go:1

  • The central transformer pipeline builder (e.g. GetScriptTransformers) appears to have been removed, so none of the individual transformers (including the new optional catch transformer) will ever be applied. You should restore or reimplement the pipeline in transformer.go to invoke newOptionalCatchTransformer and the other transforms in the correct order.
package transformers

internal/transformers/estransforms/optionalcatch.go:28

  • Using NewTempVariable() can produce unpredictable names that may not match the test baselines (which expect _a). Consider using Factory().NewGeneratedNameForNode(node) or a consistent naming scheme to match other downlevel transforms.
			ch.Factory().NewVariableDeclaration(ch.Factory().NewTempVariable(), nil, nil, nil),

@weswigham weswigham added this pull request to the merge queue Jun 24, 2025
Merged via the queue into microsoft:main with commit fa5d50b Jun 24, 2025
22 checks passed
@weswigham weswigham deleted the es-dl-catch branch June 24, 2025 01:04
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