Skip to content

feat(napi/minify): expose treeshake options#15109

Merged
graphite-app[bot] merged 1 commit intomainfrom
copilot/add-bindings-for-treeshake-option
Oct 30, 2025
Merged

feat(napi/minify): expose treeshake options#15109
graphite-app[bot] merged 1 commit intomainfrom
copilot/add-bindings-for-treeshake-option

Conversation

Copy link
Contributor

Copilot AI commented Oct 30, 2025

feat(napi/minify): expose treeshake options

Completed:

  • Understand the problem: Need to expose TreeShakeOptions to JavaScript users
  • Explore existing code structure and TreeShakeOptions in oxc_minifier
  • Build the project to ensure clean state
  • Add TreeShakeOptions struct with all fields
  • Update CompressOptions to accept optional treeshake field
  • Implement conversion from NAPI TreeShakeOptions to oxc_minifier::TreeShakeOptions
  • Build and verify TypeScript definitions are generated correctly
  • Add tests for treeshake options
  • Run all tests successfully
  • Format code with just fmt
  • Address all review feedback:
    • Renamed TreeShakeOptionsNapi to TreeShakeOptions
    • Changed property_read_side_effects to accept boolean | 'always'
    • Changed @default from true to 'always' for property_read_side_effects
    • Reverted From implementation back to use reference (From<&TreeShakeOptions>)
    • Changed to TryFrom to properly handle validation errors
    • Added validation for propertyReadSideEffects string values
    • Fixed clippy lint for inline format args

Changes Made:

  1. Added TreeShakeOptions struct with fields:
    • annotations: Whether to respect pure annotations (@PURE, #NO_SIDE_EFFECTS)
    • manual_pure_functions: List of function names to treat as pure
    • property_read_side_effects: Whether property reads have side effects (boolean | 'always', default: 'always')
    • unknown_global_side_effects: Whether accessing global variables has side effects
  2. Updated CompressOptions to include optional treeshake field
  3. Implemented TryFrom<&TreeShakeOptions> trait with proper error handling for invalid values
  4. Added validation that rejects invalid string values for propertyReadSideEffects
  5. Added tests covering treeshake functionality including error cases
  6. Fixed all lint issues

All tests pass successfully!

Original prompt

Add bindings for treeshake option here:

treeshake: TreeShakeOptions::default(),


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@graphite-app
Copy link
Contributor

graphite-app bot commented Oct 30, 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.

Copilot AI changed the title [WIP] Add bindings for treeshake option in options.rs Add NAPI bindings for treeshake options in minify Oct 30, 2025
Copilot AI requested a review from sapphi-red October 30, 2025 04:10
@sapphi-red sapphi-red changed the title Add NAPI bindings for treeshake options in minify feat(napi/minify): expose treeshake options Oct 30, 2025
@github-actions github-actions bot added A-minifier Area - Minifier C-enhancement Category - New feature or request labels Oct 30, 2025
Copilot AI changed the title feat(napi/minify): expose treeshake options Add NAPI bindings for treeshake options in minify Oct 30, 2025
Copilot AI requested a review from sapphi-red October 30, 2025 04:27
@sapphi-red sapphi-red changed the title Add NAPI bindings for treeshake options in minify feat(napi/minify): expose treeshake options Oct 30, 2025
Copilot AI requested a review from sapphi-red October 30, 2025 04:53
Copilot AI changed the title feat(napi/minify): expose treeshake options feat(napi/minify): expose treeshake options Oct 30, 2025
@sapphi-red sapphi-red marked this pull request as ready for review October 30, 2025 05:53
Copilot AI review requested due to automatic review settings October 30, 2025 05:53
@sapphi-red sapphi-red assigned Boshen and unassigned sapphi-red Oct 30, 2025
@sapphi-red sapphi-red requested a review from Boshen October 30, 2025 05:53
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 support for configuring treeshake options through the NAPI minifier interface. The changes allow users to control dead code elimination behavior by configuring annotations, manual pure functions, property read side effects, and global variable access side effects.

  • Defines a new TreeShakeOptions struct in Rust with conversion logic to the internal oxc_minifier types
  • Adds a treeshake field to the CompressOptions struct
  • Updates TypeScript type definitions with the new treeshake options interface
  • Adds comprehensive test coverage for all treeshake configuration options

Reviewed Changes

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

File Description
napi/minify/src/options.rs Adds TreeShakeOptions struct, implements conversion to oxc_minifier types, and integrates treeshake options into CompressOptions
napi/minify/index.d.ts Adds TypeScript interface definitions for TreeShakeOptions and updates CompressOptions interface
napi/minify/test/minify.test.ts Adds test suite covering treeshake options including annotations, manual pure functions, and property read side effects

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

@sapphi-red sapphi-red marked this pull request as draft October 30, 2025 06:14
@sapphi-red
Copy link
Member

@copilot change to try_from to fix the compile error

Copy link
Contributor Author

Copilot AI commented Oct 30, 2025

@copilot change to try_from to fix the compile error

Done in 15b9d67. Changed from From to TryFrom to properly handle validation errors for invalid propertyReadSideEffects values. Also added a test to verify error handling.

@sapphi-red
Copy link
Member

@copilot fix lint

Copy link
Contributor Author

Copilot AI commented Oct 30, 2025

@copilot fix lint

Done in e235a5e. Fixed clippy lint by using inline format args ('{s}' instead of '{}' with s as a separate argument).

@sapphi-red sapphi-red marked this pull request as ready for review October 30, 2025 08:12
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Oct 30, 2025
Copy link
Member

Boshen commented Oct 30, 2025

Merge activity

feat(napi/minify): expose `treeshake` options

## Completed:
- [x] Understand the problem: Need to expose TreeShakeOptions to JavaScript users
- [x] Explore existing code structure and TreeShakeOptions in oxc_minifier
- [x] Build the project to ensure clean state
- [x] Add TreeShakeOptions struct with all fields
- [x] Update CompressOptions to accept optional treeshake field
- [x] Implement conversion from NAPI TreeShakeOptions to oxc_minifier::TreeShakeOptions
- [x] Build and verify TypeScript definitions are generated correctly
- [x] Add tests for treeshake options
- [x] Run all tests successfully
- [x] Format code with just fmt
- [x] Address all review feedback:
  - Renamed TreeShakeOptionsNapi to TreeShakeOptions
  - Changed property_read_side_effects to accept boolean | 'always'
  - Changed @default from true to 'always' for property_read_side_effects
  - Reverted From implementation back to use reference (From<&TreeShakeOptions>)
  - Changed to TryFrom to properly handle validation errors
  - Added validation for propertyReadSideEffects string values
  - Fixed clippy lint for inline format args

## Changes Made:
1. Added `TreeShakeOptions` struct with fields:
   - `annotations`: Whether to respect pure annotations (@__PURE__, #__NO_SIDE_EFFECTS__)
   - `manual_pure_functions`: List of function names to treat as pure
   - `property_read_side_effects`: Whether property reads have side effects (boolean | 'always', default: 'always')
   - `unknown_global_side_effects`: Whether accessing global variables has side effects
2. Updated `CompressOptions` to include optional `treeshake` field
3. Implemented `TryFrom<&TreeShakeOptions>` trait with proper error handling for invalid values
4. Added validation that rejects invalid string values for propertyReadSideEffects
5. Added tests covering treeshake functionality including error cases
6. Fixed all lint issues

All tests pass successfully!

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

<details>

<summary>Original prompt</summary>

> Add bindings for treeshake option here:
> https://github.com/oxc-project/oxc/blob/3fbb307367a31817a297dee299fec580912675db/napi/minify/src/options.rs#L90

</details>

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

💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey).
@graphite-app graphite-app bot force-pushed the copilot/add-bindings-for-treeshake-option branch from e235a5e to 1c31cb1 Compare October 30, 2025 08:46
@graphite-app graphite-app bot merged commit 1c31cb1 into main Oct 30, 2025
18 checks passed
@graphite-app graphite-app bot deleted the copilot/add-bindings-for-treeshake-option branch October 30, 2025 08:51
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Oct 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-minifier Area - Minifier C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments