Skip to content

test(formatter): add tests for import sorting in ambient modules#22072

Merged
graphite-app[bot] merged 1 commit into
mainfrom
om/05-02-test_formatter_add_tests_for_import_sorting_in_ambient_modules
May 3, 2026
Merged

test(formatter): add tests for import sorting in ambient modules#22072
graphite-app[bot] merged 1 commit into
mainfrom
om/05-02-test_formatter_add_tests_for_import_sorting_in_ambient_modules

Conversation

@overlookmotel

@overlookmotel overlookmotel commented May 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Add tests for sorting imports within ambient modules. e.g.:

declare module "foo" {
  import InnerX from "inner-x";
  import InnerY from "inner-y";
  import InnerZ from "inner-z";
}

This is supported already, but there were no tests for it.

@overlookmotel overlookmotel mentioned this pull request May 2, 2026
Merged
@overlookmotel Graphite App

overlookmotel commented May 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

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 changes, fast-track this PR to the front of the merge queue

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.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@overlookmotel overlookmotel marked this pull request as ready for review May 2, 2026 10:44
Copilot AI review requested due to automatic review settings May 2, 2026 10:44
@overlookmotel overlookmotel added C-test Category - Testing. Code is missing test cases, or a PR is adding them A-formatter Area - Formatter labels May 2, 2026
Copilot AI reviewed May 2, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds formatter regression tests to validate that sortImports correctly sorts import declarations within TypeScript ambient module bodies (declare module "…" { … }) without affecting surrounding top-level import chunks.

Changes:

  • Register a new ambient_modules test module in the import-sorting IR transform test suite.
  • Add coverage for sorting inside a single ambient module, across mixed top-level + ambient scopes, and across multiple ambient modules.

Reviewed changes

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

File Description
crates/oxc_formatter/tests/ir_transform/sort_imports/mod.rs Adds the new ambient-module-focused test module to the suite.
crates/oxc_formatter/tests/ir_transform/sort_imports/ambient_modules.rs Introduces targeted tests ensuring import sorting is scoped correctly inside ambient modules and across chunk boundaries.

@codspeed-hq

codspeed-hq Bot commented May 2, 2026 •
edited
Loading

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 44 untouched benchmarks
⏩ 7 skipped benchmarks1

Comparing om/05-02-test_formatter_add_tests_for_import_sorting_in_ambient_modules (484b615) with main (bc1408c)

Open in CodSpeed

Footnotes

  1. 7 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. ↩

This was referenced May 2, 2026
Merged
Merged
Merged
@overlookmotel overlookmotel force-pushed the om/05-02-test_formatter_add_tests_for_import_sorting_in_ambient_modules branch from 4705d36 to 484b615 Compare May 2, 2026 12:50
leaysgur
leaysgur approved these changes May 3, 2026
View reviewed changes

@leaysgur leaysgur 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.

TIL...!

@overlookmotel

overlookmotel commented May 3, 2026

Copy link
Copy Markdown
Member Author

TIL...!

Well you implemented it, not me!

@graphite-app graphite-app Bot added the 0-merge Merge with Graphite Merge Queue label May 3, 2026
@graphite-app

graphite-app Bot commented May 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Merge activity

@overlookmotel
test(formatter): add tests for import sorting in ambient modules (#22072
2c7a2ea 
)

Add tests for sorting imports within ambient modules. e.g.:

```ts
declare module "foo" {
  import InnerX from "inner-x";
  import InnerY from "inner-y";
  import InnerZ from "inner-z";
}
```

This is supported already, but there were no tests for it.
@graphite-app graphite-app Bot force-pushed the om/05-02-test_formatter_add_tests_for_import_sorting_in_ambient_modules branch from 484b615 to 2c7a2ea Compare May 3, 2026 20:44
@graphite-app graphite-app Bot merged commit 2c7a2ea into main May 3, 2026
27 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label May 3, 2026
@graphite-app graphite-app Bot deleted the om/05-02-test_formatter_add_tests_for_import_sorting_in_ambient_modules branch May 3, 2026 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@leaysgur leaysgur leaysgur approved these changes

@Dunqing Dunqing Awaiting requested review from Dunqing Dunqing is a code owner

Assignees

@leaysgur leaysgur

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@overlookmotel @leaysgur