Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 25, 2025

PR #16114 fixed TSImportType codegen to prevent backticks in minified output (which produces invalid TypeScript). This adds regression tests to ensure the fix persists.

Changes

  • Added 4 test cases covering TSImportType variants:
    • Basic import types with single/double quotes
    • Import types with qualifiers (.SomeType)
    • Import types with type arguments (<string>)
  • Tests verify both normal and minified output via snapshots

Example

Input:

type T = typeof import('react');

Minified output (now tested):

type T=typeof import("react");  // ✓ double quotes, not backticks

The minify snapshot ensures backticks are never used, preventing invalid TS like import(\react`)`.

Original prompt

This section details on the original issue you should resolve

<issue_title>Codegen: Do not use backticks when printing argument of TSImportType</issue_title>
<issue_description>Input:

type T = typeof import('react');

Codegen output (with minify: true):

type T=typeof import(`react`);

Playground

This is invalid TypeScript. TS appears to require the argument to be a string literal, and nothing else - template literal is not allowable here.

TS Playground

Related to #16074.</issue_description>

<agent_instructions>The issue itself is solved. Add a test for it.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@overlookmotel We might want to do #16111 first. That'll make this fix easier to implement. @overlookmotel #16114 should have fixed this. We just need to add a test to make sure. @camc314 There are tests for this, they regressed after just the AST change. I fixed them back up in 16114 to avoid the regression

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 25, 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.

…n minify mode

Co-authored-by: overlookmotel <557937+overlookmotel@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix codegen to not use backticks for TSImportType test(codegen): Add regression tests for TSImportType string literal output Nov 25, 2025
Copilot AI requested a review from overlookmotel November 25, 2025 19:14
@camc314 camc314 marked this pull request as ready for review November 25, 2025 20:15
Copilot AI review requested due to automatic review settings November 25, 2025 20:15
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 adds regression tests for the TSImportType codegen fix from PR #16114, which prevented invalid TypeScript output by ensuring backticks are never used in import() type expressions. The tests verify that both normal and minified output correctly use quotes (not backticks) for TSImportType arguments.

  • Added 4 comprehensive test cases covering different TSImportType variants (basic imports, qualifiers, type arguments)
  • Tests verify both normal output (preserves original quotes) and minified output (uses double quotes, never backticks)
  • Prevents regression of the bug where minification produced invalid TypeScript like import(\react`)`

Reviewed changes

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

File Description
crates/oxc_codegen/tests/integration/ts.rs Adds 4 test cases for TSImportType with different quote styles, qualifiers, and type arguments
crates/oxc_codegen/tests/integration/snapshots/ts.snap Snapshot showing normal output preserves original quote styles
crates/oxc_codegen/tests/integration/snapshots/minify.snap Snapshot verifying minified output uses double quotes (not backticks) for all TSImportType cases

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

Copy link
Member

@overlookmotel overlookmotel left a comment

Choose a reason for hiding this comment

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

Thanks robot!

@github-actions github-actions bot added A-codegen Area - Code Generation C-test Category - Testing. Code is missing test cases, or a PR is adding them labels Nov 25, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Nov 25, 2025

CodSpeed Performance Report

Merging #16120 will not alter performance

Comparing copilot/fix-codegen-backticks (30cfa8d) with main (74cf572)

Summary

✅ 42 untouched
⏩ 3 skipped1

Footnotes

  1. 3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@overlookmotel overlookmotel merged commit 5233d73 into main Nov 25, 2025
42 of 55 checks passed
@overlookmotel overlookmotel deleted the copilot/fix-codegen-backticks branch November 25, 2025 22:44
leaysgur pushed a commit that referenced this pull request Nov 26, 2025
…utput (#16120)

PR #16114 fixed TSImportType codegen to prevent backticks in minified
output (which produces invalid TypeScript). This adds regression tests
to ensure the fix persists.

## Changes

- Added 4 test cases covering TSImportType variants:
  - Basic import types with single/double quotes
  - Import types with qualifiers (`.SomeType`)
  - Import types with type arguments (`<string>`)
- Tests verify both normal and minified output via snapshots

## Example

Input:
```typescript
type T = typeof import('react');
```

Minified output (now tested):
```typescript
type T=typeof import("react");  // ✓ double quotes, not backticks
```

The minify snapshot ensures backticks are never used, preventing invalid
TS like `import(\`react\`)`.

<!-- START COPILOT CODING AGENT SUFFIX -->



<details>

<summary>Original prompt</summary>

> 
> ----
> 
> *This section details on the original issue you should resolve*
> 
> <issue_title>Codegen: Do not use backticks when printing argument of
`TSImportType`</issue_title>
> <issue_description>Input:
> 
> ```ts
> type T = typeof import('react');
> ```
> 
> Codegen output (with `minify: true`):
> 
> ```ts
> type T=typeof import(`react`);
> ```
> 
>
[Playground](https://playground.oxc.rs/#eNptU01rGzEQ/StGl7TgQhooBJceSlJDIW3SJjSXXma1s44SrUYdzcY2xv+9o/VKTiCn1b7RfLw3TztjzcLINuLsbvZllg/UzVwfieXdCSNYOXn/+W8wc0NmsTM8hPzxLohZdOATzk1H3IMIckWEIaQMV8Ql8iDYXqL1wCCOQqrB9YMTTBEsmoXwoEgPYeX1b7pgqY+M6ZiRLMV6OW37hnz5s91qurafmwic8lw7gxvBkLRtppuUDnhP698oA4frQZJrcTkEmwcrlXJL5Ge8AdbUgo55f86/B2GnBW0NJNSpxdlvzMQTqiNkqcYR9PxCqZ0ZEt5Bc+Qk0Ny7Vh7M4mxuMLTX3ZULStL4TseN2k2m+PmpdnNZoV8DyVGmx7S5fQP+l/9umKL2MpA+BMQWW62pa3I63uqC+h5yzHtFlYerZJXLuvDX+Bq2WbuGwT6h3OrKNL1cLij0eJh86k/NI1q5Z4hao4iaG42zfhUVshl0QuRXabiJyK7HIOCv9QhCfEPJHVaUFTIqabXaQVQBXqFa02A6O/34SbuozJfYaeEl8YWHlJYOfVu39rKLmpPGNsdo76SiP1CgBVGlptW+beqdScooqkGQA6gvixkttbjC8f2EbIRq2cfUkj7DSfIQSMZaBfG4KnfHKuUtZEeVh6LMKV7hM9aiT4jxp66iGGxMVdOSX6qBc8YzckOpCJ75hLyoHKqnvcLtKF+G62mf8Sxz4bT/D3SBh38=)
> 
> This is invalid TypeScript. TS appears to require the argument to be a
string literal, and nothing else - template literal is not allowable
here.
> 
> [TS
Playground](https://www.typescriptlang.org/play/?#code/C4TwDgpgBAglC8VSQPYDMoEsC2YUCdgAKAA3wgEMBjYEgSgG4AoIA)
> 
> Related to #16074.</issue_description>
> 
> <agent_instructions>The issue itself is solved. Add a test for
it.</agent_instructions>
> 
> ## Comments on the Issue (you are @copilot in this section)
> 
> <comments>
> <comment_new><author>@overlookmotel</author><body>
> We might want to do #16111 first. That'll make this fix
easier to implement.</body></comment_new>
> <comment_new><author>@overlookmotel</author><body>
> #16114 should have fixed this. We just need to add a
test to make sure.</body></comment_new>
> <comment_new><author>@camc314</author><body>
> There are tests for this, they regressed after just the AST change. I
fixed them back up in 16114 to avoid the regression</body></comment_new>
> </comments>
> 


</details>

- Fixes #16078

<!-- START COPILOT CODING AGENT TIPS -->
---

✨ Let Copilot coding agent [set things up for
you](https://github.com/oxc-project/oxc/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot)
— coding agent works faster and does higher quality work when set up for
your repo.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: overlookmotel <557937+overlookmotel@users.noreply.github.com>
taearls pushed a commit to taearls/oxc that referenced this pull request Dec 11, 2025
…utput (oxc-project#16120)

PR oxc-project#16114 fixed TSImportType codegen to prevent backticks in minified
output (which produces invalid TypeScript). This adds regression tests
to ensure the fix persists.

## Changes

- Added 4 test cases covering TSImportType variants:
  - Basic import types with single/double quotes
  - Import types with qualifiers (`.SomeType`)
  - Import types with type arguments (`<string>`)
- Tests verify both normal and minified output via snapshots

## Example

Input:
```typescript
type T = typeof import('react');
```

Minified output (now tested):
```typescript
type T=typeof import("react");  // ✓ double quotes, not backticks
```

The minify snapshot ensures backticks are never used, preventing invalid
TS like `import(\`react\`)`.

<!-- START COPILOT CODING AGENT SUFFIX -->



<details>

<summary>Original prompt</summary>

> 
> ----
> 
> *This section details on the original issue you should resolve*
> 
> <issue_title>Codegen: Do not use backticks when printing argument of
`TSImportType`</issue_title>
> <issue_description>Input:
> 
> ```ts
> type T = typeof import('react');
> ```
> 
> Codegen output (with `minify: true`):
> 
> ```ts
> type T=typeof import(`react`);
> ```
> 
>
[Playground](https://playground.oxc.rs/#eNptU01rGzEQ/StGl7TgQhooBJceSlJDIW3SJjSXXma1s44SrUYdzcY2xv+9o/VKTiCn1b7RfLw3TztjzcLINuLsbvZllg/UzVwfieXdCSNYOXn/+W8wc0NmsTM8hPzxLohZdOATzk1H3IMIckWEIaQMV8Ql8iDYXqL1wCCOQqrB9YMTTBEsmoXwoEgPYeX1b7pgqY+M6ZiRLMV6OW37hnz5s91qurafmwic8lw7gxvBkLRtppuUDnhP698oA4frQZJrcTkEmwcrlXJL5Ge8AdbUgo55f86/B2GnBW0NJNSpxdlvzMQTqiNkqcYR9PxCqZ0ZEt5Bc+Qk0Ny7Vh7M4mxuMLTX3ZULStL4TseN2k2m+PmpdnNZoV8DyVGmx7S5fQP+l/9umKL2MpA+BMQWW62pa3I63uqC+h5yzHtFlYerZJXLuvDX+Bq2WbuGwT6h3OrKNL1cLij0eJh86k/NI1q5Z4hao4iaG42zfhUVshl0QuRXabiJyK7HIOCv9QhCfEPJHVaUFTIqabXaQVQBXqFa02A6O/34SbuozJfYaeEl8YWHlJYOfVu39rKLmpPGNsdo76SiP1CgBVGlptW+beqdScooqkGQA6gvixkttbjC8f2EbIRq2cfUkj7DSfIQSMZaBfG4KnfHKuUtZEeVh6LMKV7hM9aiT4jxp66iGGxMVdOSX6qBc8YzckOpCJ75hLyoHKqnvcLtKF+G62mf8Sxz4bT/D3SBh38=)
> 
> This is invalid TypeScript. TS appears to require the argument to be a
string literal, and nothing else - template literal is not allowable
here.
> 
> [TS
Playground](https://www.typescriptlang.org/play/?#code/C4TwDgpgBAglC8VSQPYDMoEsC2YUCdgAKAA3wgEMBjYEgSgG4AoIA)
> 
> Related to oxc-project#16074.</issue_description>
> 
> <agent_instructions>The issue itself is solved. Add a test for
it.</agent_instructions>
> 
> ## Comments on the Issue (you are @copilot in this section)
> 
> <comments>
> <comment_new><author>@overlookmotel</author><body>
> We might want to do oxc-project#16111 first. That'll make this fix
easier to implement.</body></comment_new>
> <comment_new><author>@overlookmotel</author><body>
> oxc-project#16114 should have fixed this. We just need to add a
test to make sure.</body></comment_new>
> <comment_new><author>@camc314</author><body>
> There are tests for this, they regressed after just the AST change. I
fixed them back up in 16114 to avoid the regression</body></comment_new>
> </comments>
> 


</details>

- Fixes oxc-project#16078

<!-- START COPILOT CODING AGENT TIPS -->
---

✨ Let Copilot coding agent [set things up for
you](https://github.com/oxc-project/oxc/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot)
— coding agent works faster and does higher quality work when set up for
your repo.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: overlookmotel <557937+overlookmotel@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-codegen Area - Code Generation C-test Category - Testing. Code is missing test cases, or a PR is adding them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Codegen: Do not use backticks when printing argument of TSImportType

3 participants