Skip to content

Conversation

@Boshen
Copy link
Member

@Boshen Boshen commented Oct 15, 2025

Summary

Add a changed: bool field to ReplaceGlobalDefinesReturn to track whether the AST was modified during the global defines replacement transformation.

  • Adds changed field to ReplaceGlobalDefinesReturn struct
  • Tracks changes via consolidated mark_as_changed() helper method
  • Updates both expression and assignment expression traversal methods
  • Adds test assertion to verify correctness of the changed field

This allows callers to efficiently determine if replacements were made without needing to compare the AST before and after transformation.

Test plan

  • All existing tests pass
  • Added assertion in test helper to verify changed field matches expected behavior (source_text != expected)

🤖 Generated with Claude Code

@Boshen Boshen requested a review from overlookmotel as a code owner October 15, 2025 06:10
Copilot AI review requested due to automatic review settings October 15, 2025 06:10
@Boshen Boshen requested a review from Dunqing as a code owner October 15, 2025 06:10
@graphite-app
Copy link
Contributor

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

@github-actions github-actions bot added the C-enhancement Category - New feature or request label Oct 15, 2025
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 a changed field to ReplaceGlobalDefinesReturn to track whether the AST was modified during the global defines replacement transformation, eliminating the need for AST comparison to detect changes.

Key Changes

  • Added changed: bool field to ReplaceGlobalDefinesReturn struct with tracking logic
  • Implemented mark_as_changed() helper method called when replacements occur
  • Added test assertion to validate the changed field accuracy

Reviewed Changes

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

File Description
crates/oxc_transformer_plugins/src/replace_global_defines.rs Adds changed field to struct and return type, implements tracking via mark_as_changed() method
crates/oxc_transformer_plugins/tests/integrations/replace_global_defines.rs Adds assertion to verify changed field correctness and updates test cases to use transforming replacements

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

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

Boshen commented Oct 15, 2025

Merge activity

…eturn (#14617)

## Summary

Add a `changed: bool` field to `ReplaceGlobalDefinesReturn` to track whether the AST was modified during the global defines replacement transformation.

- Adds `changed` field to `ReplaceGlobalDefinesReturn` struct
- Tracks changes via consolidated `mark_as_changed()` helper method
- Updates both expression and assignment expression traversal methods
- Adds test assertion to verify correctness of the `changed` field

This allows callers to efficiently determine if replacements were made without needing to compare the AST before and after transformation.

## Test plan

- [x] All existing tests pass
- [x] Added assertion in test helper to verify `changed` field matches expected behavior (`source_text != expected`)

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@graphite-app graphite-app bot force-pushed the feat/replace-global-defines-changed-field branch from dd1a873 to a81267e Compare October 15, 2025 06:17
@graphite-app graphite-app bot merged commit a81267e into main Oct 15, 2025
20 checks passed
@graphite-app graphite-app bot deleted the feat/replace-global-defines-changed-field branch October 15, 2025 06:22
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Oct 15, 2025
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