fix(ast_tools): oxc_ast_tools depend on local oxc_* crates
#15565
+314
−538
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
How to use the Graphite Merge QueueAdd either label to this PR to merge it via 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. |
Copilot
AI
changed the title
[WIP] Remove dependency of ast_tools on Oxc crates from crates.io
Break circular dependency of Nov 10, 2025
ast_tools on Oxc crates from crates.io
Copilot finished work on behalf of
overlookmotel
November 10, 2025 19:45
overlookmotel
changed the title
ast_tools on Oxc crates from crates.iooxc_ast_tools depend on local oxc_* crates
Copilot finished reviewing on behalf of
overlookmotel
November 11, 2025 10:46
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a circular dependency issue where oxc_ast_tools depends on crates.io versions of oxc_* crates (v0.96.0), which prevents compilation when local AST changes break those crates before codegen can run to fix them.
Key changes:
- Adds a
generate-jsfeature flag (enabled by default) to make JavaScript generators optional - Makes all
oxc_*crate dependencies optional, activated only when the feature is enabled - Implements fallback logic in the
just astcommand to retry without JS generators if the first attempt fails
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tasks/ast_tools/Cargo.toml | Adds generate-js feature flag and makes all oxc_* dependencies optional |
| tasks/ast_tools/src/main.rs | Adds conditional compilation for JS generators and updates comments |
| tasks/ast_tools/src/generators/mod.rs | Gates JS generator module declarations and exports behind feature flag |
| tasks/ast_tools/src/output/mod.rs | Gates Javascript output variant and related functions behind feature flag |
| justfile | Updates ast command with fallback logic to handle compilation failures |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
overlookmotel
force-pushed
the
copilot/remove-ast-tools-dependency
branch
from
cb8292c to
cacd59d
Compare
CodSpeed Performance ReportMerging #15565 will not alter performanceComparing Summary
Footnotes |
overlookmotel
approved these changes
Nov 11, 2025
Member
|
@Boshen please see my comment: #15564 (comment) If you agree with the rationale, I think this PR is good to go. It means we no longer have dependencies on our own crates from crates.io. |
overlookmotel
force-pushed
the
copilot/remove-ast-tools-dependency
branch
from
cacd59d to
c30ef5a
Compare
Member
Merge activity
|
Fix #15564. `oxc_ast_tools` depends on Oxc crates from crates.io, not the local workspace. When AST changes break local crates, the codegen can't compile either - a Catch-22 situation, since running codegen is the only way to fix compilation. This isn't ideal as it complicates dependency management. Alter `oxc_ast_tools` so it depends on crates from local workspace, and work around the Catch-22 problem by introducing a `generate-js` Cargo feature which disables these dependencies, so the codegen can still compile if they're broken. ## Changes - **Add `generate-js` Cargo feature** (enabled by default) - Makes all `oxc_*` dependencies optional, activated only when feature enabled. - Only JS generators (`RawTransferGenerator`, `TypescriptGenerator`, `ESTreeVisitGenerator`, `RawTransferLazyGenerator`) require these dependencies. - **Conditional compilation** - Gate JS generator modules and `Output::Javascript` variant behind `#[cfg(feature = "generate-js")]` - **Fallback in `just ast`** ```sh cargo run -p oxc_ast_tools || ( cargo run -p oxc_ast_tools --no-default-features && cargo run -p oxc_ast_tools ) ``` First attempts to run full codegen. On failure, regenerates Rust code only, without dependencies (so that `oxc_*` crates will now compile), then retries full generation.
graphite-app
bot
force-pushed
the
copilot/remove-ast-tools-dependency
branch
from
c30ef5a to
6d4efdd
Compare
This was referenced Nov 11, 2025
graphite-app bot
pushed a commit
that referenced
this pull request
Nov 11, 2025
#15603) #15565 made `oxc_ast_tools` depend on local workspace versions of various crates. Add those crates to the watch list for "AST Changes" CI task, so if changes are made in them, `ast_tools` codegen will run to ensure generated code has been updated to reflect those changes. In particular, this will prevent generated code getting out of sync when changes to minifier affect the generated code.
graphite-app bot
pushed a commit
that referenced
this pull request
Nov 14, 2025
According to Claude, `just ast` in its current form will not work on Windows, as PowerShell does not support `( ... )` for command groupings. Switch to `{ ... }`.
This script was added in #15565. Copilot writes it, Claude critiques it!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #15564.
oxc_ast_toolsdepends on Oxc crates from crates.io, not the local workspace. When AST changes break local crates, the codegen can't compile either - a Catch-22 situation, since running codegen is the only way to fix compilation.This isn't ideal as it complicates dependency management. Alter
oxc_ast_toolsso it depends on crates from local workspace, and work around the Catch-22 problem by introducing agenerate-jsCargo feature which disables these dependencies, so the codegen can still compile if they're broken.Changes
Add
generate-jsCargo feature (enabled by default)oxc_*dependencies optional, activated only when feature enabled.RawTransferGenerator,TypescriptGenerator,ESTreeVisitGenerator,RawTransferLazyGenerator) require these dependencies.Conditional compilation
Output::Javascriptvariant behind#[cfg(feature = "generate-js")]Fallback in
just astFirst attempts to run full codegen. On failure, regenerates Rust code only, without dependencies (so that
oxc_*crates will now compile), then retries full generation.