Skip to content

fix(transformer): Always update jsx options from comments#10966

Merged
Dunqing merged 2 commits intooxc-project:mainfrom
magic-akari:fix/issue-10956
May 12, 2025
Merged

fix(transformer): Always update jsx options from comments#10966
Dunqing merged 2 commits intooxc-project:mainfrom
magic-akari:fix/issue-10956

Conversation

@magic-akari
Copy link
Contributor

@magic-akari magic-akari commented May 12, 2025

Note

The explanation in the comment is incorrect because the h function might be locally defined rather than imported. Removing the conditional check will fix the issue.

@graphite-app
Copy link
Contributor

graphite-app bot commented May 12, 2025

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

@github-actions github-actions bot added A-transformer Area - Transformer / Transpiler C-bug Category - Bug labels May 12, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented May 12, 2025

CodSpeed Instrumentation Performance Report

Merging #10966 will not alter performance

Comparing magic-akari:fix/issue-10956 (8634920) with main (e31c361)

Summary

✅ 36 untouched benchmarks

@magic-akari magic-akari marked this pull request as ready for review May 12, 2025 13:08
Copilot AI review requested due to automatic review settings May 12, 2025 13:08
Copy link
Contributor

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

This PR fixes an issue where JSX options were not correctly updated from comments due to an overly strict conditional check. Key changes include:

  • Removing the additional conditional check for TypeScript's only_remove_type_imports in favor of a simpler check for JSX.
  • Updating related test fixtures and snapshots to reflect the new behavior.
  • Adjusting transformer logic to always update JSX options from comments.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tasks/transform_conformance/tests/babel-plugin-transform-typescript/test/fixtures/jsx/issue-10956/output.js Updated output test to use the local h function for JSX.
tasks/transform_conformance/tests/babel-plugin-transform-typescript/test/fixtures/jsx/issue-10956/options.json Updated options to include necessary plugins.
tasks/transform_conformance/tests/babel-plugin-transform-typescript/test/fixtures/jsx/issue-10956/input.tsx Uses JSX pragmas appropriately with classic runtime mode.
tasks/transform_conformance/snapshots/oxc.snap.md Updated snapshots reflecting the new behavior in transformer tests.
crates/oxc_transformer/src/lib.rs Simplified conditional check for updating JSX options from comments.

Copy link
Member

@Dunqing Dunqing left a comment

Choose a reason for hiding this comment

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

Thanks!

@Dunqing Dunqing added the 0-merge Merge with Graphite Merge Queue label May 12, 2025
@graphite-app
Copy link
Contributor

graphite-app bot commented May 12, 2025

Merge activity

  • May 12, 11:45 AM EDT: @magic-akari we removed the merge queue label because we could not find a Graphite account associated with your GitHub profile.

You must have a Graphite account in order to use the merge queue. Create an account and try again using this link

@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label May 12, 2025
@Dunqing Dunqing merged commit 6c20277 into oxc-project:main May 12, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-transformer Area - Transformer / Transpiler C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

transformer: @jsxRuntime pragma comment is ignored when typescript.onlyRemoveTypeImports: true is set

3 participants