Skip to content

Conversation

@leaysgur
Copy link
Member

@leaysgur leaysgur commented Oct 21, 2025

#14803 (comment)

.d.ts was parsed as .ts by path.extension().

Copilot AI review requested due to automatic review settings October 21, 2025 01:29
@leaysgur leaysgur requested a review from Dunqing as a code owner October 21, 2025 01:29
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 the handling of .d.ts (TypeScript declaration) files in the oxfmt formatter. The key change is switching from extension-based detection to path-based detection to properly recognize .d.ts files, which have a compound extension.

  • Changed SourceType::from_extension() to SourceType::from_path() to correctly detect .d.ts files
  • Added test coverage for .d.ts files and glob pattern exclusions
  • Added multiple test fixture files to verify formatting behavior across different file types

Reviewed Changes

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

Show a summary per file
File Description
crates/oxc_formatter/src/service/source_type.rs Switched from extension-based to path-based source type detection to handle .d.ts files correctly
apps/oxfmt/tests/mod.rs Added test case for glob pattern exclusion of TypeScript files
apps/oxfmt/tests/snapshots/multiple_files@oxfmt.snap.new Updated test snapshot to include .d.ts file and new glob exclusion test case
apps/oxfmt/tests/fixtures/multiple_files/simple.ts Added new TypeScript test fixture with formatting issues
apps/oxfmt/tests/fixtures/multiple_files/component.jsx Added new JSX test fixture with formatting issues
apps/oxfmt/tests/fixtures/multiple_files/arrow.js Updated JavaScript test fixture content from arrow functions to JSX element
apps/oxfmt/tests/fixtures/multiple_files/app.tsx Added new TSX test fixture with formatting issues

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
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 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.

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

@github-actions github-actions bot added A-cli Area - CLI A-formatter Area - Formatter C-bug Category - Bug labels Oct 21, 2025
@leaysgur leaysgur force-pushed the 10-21-fix_oxfmt_handle_.d.ts_file_correctly branch from 31b5c05 to a0e70f2 Compare October 21, 2025 01:31
@leaysgur leaysgur changed the title fix(oxfmt): Handle .d.ts file correctly fix(oxfmt): Handle .d.ts file correctly Oct 21, 2025
@leaysgur leaysgur mentioned this pull request Oct 21, 2025
5 tasks
@codspeed-hq
Copy link

codspeed-hq bot commented Oct 21, 2025

CodSpeed Performance Report

Merging #14835 will not alter performance

Comparing 10-21-fix_oxfmt_handle_.d.ts_file_correctly (a0e70f2) with main (a64180e)

Summary

✅ 33 untouched
⏩ 4 skipped1

Footnotes

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

@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Oct 21, 2025
Copy link
Member

Boshen commented Oct 21, 2025

Merge activity

> #14803 (comment)

`.d.ts` was parsed as `.ts` by `path.extension()`.
@graphite-app graphite-app bot force-pushed the 10-21-fix_oxfmt_handle_.d.ts_file_correctly branch from a0e70f2 to 7a420a1 Compare October 21, 2025 02:30
@graphite-app graphite-app bot merged commit 7a420a1 into main Oct 21, 2025
20 checks passed
@graphite-app graphite-app bot deleted the 10-21-fix_oxfmt_handle_.d.ts_file_correctly branch October 21, 2025 02:36
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Oct 21, 2025
@Boshen Boshen mentioned this pull request Oct 21, 2025
graphite-app bot pushed a commit that referenced this pull request Oct 21, 2025
## [0.7.0] - 2025-10-21

### 🚀 Features

- 6dfcd80 oxfmt: Search both .json and .jsonc config file (#14848) (leaysgur)
- aa024d9 formatter: Wrap parenthesis for `AssignmentExpression` that are inside `ComputedMemberExpression` (#14834) (Dunqing)

### 🐛 Bug Fixes

- 88fb768 formatter: Correct handling of ignore comment for `TSUnionType` and `TSMappedType` (#14824) (Dunqing)
- f7727c7 formatter: Ignore comment doesn't work for the expression statement (#14817) (Dunqing)
- 7a420a1 oxfmt: Handle `.d.ts` file correctly (#14835) (leaysgur)

### 🚜 Refactor

- 6fa7420 oxfmt: Use custom ignore builder (#14850) (leaysgur)
- 9d914a3 formatter: Improve comments handling (#14816) (Dunqing)
- f52863d formatter: Improve handling of type cast node (#14815) (Dunqing)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cli Area - CLI A-formatter Area - Formatter C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants