Skip to content

feat(span): extract TS declaration file check to its own function#17037

Merged
graphite-app[bot] merged 1 commit intomainfrom
add-ts-declaration-function
Dec 18, 2025
Merged

feat(span): extract TS declaration file check to its own function#17037
graphite-app[bot] merged 1 commit intomainfrom
add-ts-declaration-function

Conversation

@camchenry
Copy link
Member

@camchenry camchenry commented Dec 18, 2025

Extracted the is_dts check that was internal to SourceType::from_path to be its own function at FileExtension::is_ts_declaration. This allows us to reuse the declaration file extension checking code outside of just oxc_span in lint rules, for example.

Since I exported FileExtension from oxc_span, it required adding documentation for some variants and also changing from_str to be an implementation of the FromStr trait instead. This entailed some refactoring because from_str now returns Result instead of Option.

@github-actions github-actions bot added the C-enhancement Category - New feature or request label Dec 18, 2025
Copy link
Member Author

camchenry commented Dec 18, 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.

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

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 18, 2025

CodSpeed Performance Report

Merging #17037 will not alter performance

Comparing add-ts-declaration-function (328bbcf) with main (4d389f7)1

Summary

✅ 42 untouched
⏩ 3 skipped2

Footnotes

  1. No successful run was found on main (36c2853) during the generation of this report, so 4d389f7 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

  2. 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.

@camchenry camchenry requested a review from camc314 December 18, 2025 03:12
@camchenry camchenry marked this pull request as ready for review December 18, 2025 03:12
Copilot AI review requested due to automatic review settings December 18, 2025 03:12
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 extracts the TypeScript declaration file detection logic from SourceType::from_path into a reusable public method FileExtension::is_ts_declaration. This enables the declaration file checking logic to be used in other parts of the codebase, such as lint rules, without being confined to the oxc_span module's internal path processing.

Key changes:

  • Made FileExtension enum public with documentation for all variants
  • Converted FileExtension::from_str to implement the standard FromStr trait, returning Result instead of Option
  • Extracted declaration file detection into a new public method is_ts_declaration that handles .ts, .mts, and .cts extensions with arbitrary extension support for .ts files

Reviewed changes

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

File Description
crates/oxc_span/src/source_type.rs Made FileExtension enum public with docs, added is_ts_declaration method, implemented FromStr trait, refactored from_path and from_extension to use the new API, added comprehensive tests for the new method
crates/oxc_span/src/lib.rs Exported FileExtension from the module's public API

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

@camc314 camc314 self-assigned this Dec 18, 2025
@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label Dec 18, 2025
Copy link
Contributor

camc314 commented Dec 18, 2025

Merge activity

graphite-app bot pushed a commit that referenced this pull request Dec 18, 2025
…7037)

Extracted the `is_dts` check that was internal to `SourceType::from_path` to be its own function at `FileExtension::is_ts_declaration`. This allows us to reuse the declaration file extension checking code outside of just `oxc_span` in lint rules, for example.

Since I exported `FileExtension` from `oxc_span`, it required adding documentation for some variants and also changing `from_str` to be an implementation of the `FromStr` trait instead. This entailed some refactoring because `from_str` now returns `Result` instead of `Option`.
@graphite-app graphite-app bot force-pushed the add-ts-declaration-function branch from 328bbcf to 8607b25 Compare December 18, 2025 09:13
…7037)

Extracted the `is_dts` check that was internal to `SourceType::from_path` to be its own function at `FileExtension::is_ts_declaration`. This allows us to reuse the declaration file extension checking code outside of just `oxc_span` in lint rules, for example.

Since I exported `FileExtension` from `oxc_span`, it required adding documentation for some variants and also changing `from_str` to be an implementation of the `FromStr` trait instead. This entailed some refactoring because `from_str` now returns `Result` instead of `Option`.
@graphite-app graphite-app bot force-pushed the add-ts-declaration-function branch from 8607b25 to 5586823 Compare December 18, 2025 09:18
@graphite-app graphite-app bot merged commit 5586823 into main Dec 18, 2025
20 checks passed
@graphite-app graphite-app bot deleted the add-ts-declaration-function branch December 18, 2025 09:24
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Dec 18, 2025
overlookmotel added a commit that referenced this pull request Dec 19, 2025
### 🚀 Features

- d209c21 allocator: Add cap to FixedSizeAllocatorPool and block when
exhausted (#17023) (Cameron)
- fb2af91 allocator: Add bitset utils (#17042) (zhaoting zhou)
- c16082c tasks/compat_data: Integrate `node-compat-table` (#16831)
(Boshen)
- 5586823 span: Extract TS declaration file check to its own function
(#17037) (camchenry)
- 3d2b492 minifier: Fold iife arrow functions in call expressions
(#16477) (Armano)
- 67e9f9e codegen: Keep comments on the export specifiers (#16943)
(夕舞八弦)
- cb515fa parser: Improve error message for `yield` as identifier usage
(#16950) (sapphi-red)
- dcc856b parser: Add help for `new_dynamic_import` error (#16949)
(sapphi-red)
- c3c79f8 parser: Improve import attribute value error message (#16948)
(sapphi-red)
- 291b57b ast_tools: Generate TS declaration files for deserializer and
walk files (#16912) (camc314)
- 74eae13 minifier: Remove unused import specifiers (#16797) (camc314)

### 🐛 Bug Fixes

- fb9e193 linter: OOM problems with custom plugins (#17082)
(overlookmotel)
- e59132b parser/napi: Fix lazy deser (#17069) (overlookmotel)
- a92faf0 ast_tools: Support `u128` in `assert_layouts` generator
(#17050) (overlookmotel)
- 47b4c2f minifier/docs: Correct hyperlink path in OPTIMIZATIONS.md
(#16986) (GRK)
- 3002649 transformer/typescript: Remove unused import equals
declaration (#16776) (Dunqing)
- 5a2af88 regular_expression: Correct named capture group reference
error (#16952) (sapphi-red)

### ⚡ Performance

- b657bb6 allocator: Reduce time `Mutex` lock is held in
`FixedSizeAllocatorPool::get` (#17079) (overlookmotel)
- 1f3b19b ast: `#[ast]` macro use `#[repr(transparent)]` for
single-field structs (#17052) (overlookmotel)
- 225f229 parser: Use SmallVec for duplicate default export detection
(#16801) (camc314)

### 📚 Documentation

- a9c419f traverse: Update safety comments (#16944) (overlookmotel)

Co-authored-by: overlookmotel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments