feat(ast_tools): output typescript to a separate package #6755

ottomated · 2024-10-21T21:30:28Z

Part of #6347.

Moves typescript logic from derive_estree into a new ast_tools generator.

graphite-app · 2024-10-21T21:30:32Z

Your org has enabled the Graphite merge queue for merging into main

Add the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

ottomated · 2024-10-21T21:30:47Z

feat(napi/parser)!: add typings to napi/parser #6796
feat(ast_tools): output typescript to a separate package #6755 👈
main

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

Join @ottomated and the rest of your teammates on Graphite

Boshen · 2024-10-22T01:09:55Z

👍 nice work

Boshen · 2024-10-22T01:12:06Z

Our ts types came from typescript-eslint, I would model them as such https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/ast-spec/src/expression/ArrayExpression/spec.ts

but this is for another topic: #6347 (comment)

codspeed-hq · 2024-10-22T12:27:44Z

CodSpeed Performance Report

Merging #6755 will not alter performance

_{Comparing 10-21-feat_output_typescript_to_a_separate_package (1145341) with main (1c27a2c)}

Summary

✅ 30 untouched benchmarks

overlookmotel · 2024-10-22T13:58:17Z

I have taken the liberty of combining the changes from #6687 into this PR, since neither PR stands alone, and it was confusing trying to review each in isolation. I've preserved the separate commits from both PRs.

I've added a commit to remove unused dependencies, which unblocks the "autofix" CI job.

@ottomated You will need to force pull the branch to your local machine, and then run gt track to get Graphite back in sync.

This is great, but there are a few problems:

Autofix

The "autofix" CI job automatically reformats all .d.ts files in the repo with dprint. But the "AST changes" CI job checks that generated files have not been altered manually. So it's freaking out because it sees the changes "autofix" made.

Longer term we should run dprint on the .d.ts in ast_tools, so it's formatted in standard style. But that can come later. Simplest way to solve the problem for now is to add the generated .d.ts file to autofix's ignore list:

oxc/.github/workflows/autofix.yml

Lines 4 to 8 in d2b8f50

    
           pull_request: 
        
             types: [opened, synchronize] 
        
             paths-ignore: 
        
               - "!.github/workflows/ci.yml" 
        
               - "!.github/actions/clone-submodules/action.yml"

Then re-run just ast locally to regenerate the .d.ts with original formatting, and push the changes.

WASM types

This PR generates the .d.ts file, but doesn't link it into wasm-bindgen. So the types are now gone from the WASM package. This breaks that package, so this PR can't be merged as it stands (CI always needs to be green).

I suggest in this PR:

Remove the new oxc-types package.
Instead output the combined types in an .rs file as a single const TS_APPEND_CONTENT statement.
Import that module into oxc_ast/src/serialize.rs.

That'll get this PR into a state in which it can be merged. Leave figuring out how to share the same type defs between oxc-types and WASM package to the next PR. At that point, re-introduce the oxc-types package.

Missing custom types

The generated .d.ts file is missing type defs for FormalParameterRest, JSXElementName etc which are currently defined in const TS_APPEND_CONTENT statements. I know you're aware of this, but it needs to be resolved before we can switch over to the generated .d.ts.

Sorry if this all feels like bureaucracy! It's annoying sometimes, but each individual PR (even in a stack) should not leave any crates / packages in a broken state. The advantage of stacked PRs is you can move fast (no need to wait for previous PR to get merged before starting on the next one), but the trade-off is that you need to consider the order you do things in, to keep CI green on every PR.

ottomated · 2024-10-22T15:33:46Z

@overlookmotel thanks for the PR juggling, I'm well aware that this PR is not done and have marked it as draft for clarity.

ottomated · 2024-10-22T21:48:46Z

@overlookmotel everything should be addressed now!

overlookmotel

All looks good. Very nice! My comments below are just nits.

The one remaining question is the name of the types package. We don't control the @oxc npm scope unfortunately, and Oxc's NPM packages are scattered around a bit.

We could use either a scope-less package name oxc-types or @oxc-project/types.

@Boshen?

tasks/ast_tools/src/generators/typescript.rs

overlookmotel · 2024-10-23T17:50:52Z

@Boshen I've reviewed all the Rust code, but could you please take a look at the package publishing + github workflow stuff?

Also, see above my question about package name for the types. oxc-types or @oxc-project/types?

Boshen · 2024-10-24T00:40:45Z

@Boshen I've reviewed all the Rust code, but could you please take a look at the package publishing + github workflow stuff?

Also, see above my question about package name for the types. oxc-types or @oxc-project/types?

I'll address these after this PR is merged.

Boshen · 2024-10-24T00:43:07Z

Let me make a new release for oxc before merging this.

ottomated · 2024-10-24T02:19:17Z

Could also be @oxc-ast/types or something along those lines

crates/oxc_ast/src/ast/types.d.ts

overlookmotel

Boshen said he'd tackle the package release stuff after this is merged. Oxc 0.33.0 has been released. So I think this is good to merge now. Ditto #6796 and #6836.

@Boshen I'll leave it to you to merge, in case I've missing something.

graphite-app · 2024-10-24T13:08:08Z

Merge activity

Oct 24, 9:08 AM EDT: A user added this pull request to the Graphite merge queue.
Oct 24, 9:13 AM EDT: A user merged this pull request with the Graphite merge queue.

Part of #6347. Moves typescript logic from derive_estree into a new ast_tools generator.

## [0.34.0] - 2024-10-26 - 4618aa2 transformer: [**BREAKING**] Rename `TransformerOptions::react` to `jsx` (#6888) (Boshen) - 90c786c regular_expression: [**BREAKING**] Support ES2025 Duplicated named capture groups (#6847) (leaysgur) - 67a7bde napi/parser: [**BREAKING**] Add typings to napi/parser (#6796) (ottomated) ### Features - 1145341 ast_tools: Output typescript to a separate package (#6755) (ottomated) - 4429754 ecmascript: Constant eval `null` to number (#6879) (Boshen) - fd57e00 ecmascript: Add abstract_relational_comparison to dce (#6846) (Boshen) - 8bcaf59 minifier: Late peeophole optimization (#6882) (Boshen) - 860cbca minifier: Implement folding simple arrow fns (#6875) (camc314) - c26020e minifier: Implement folding String.prototype.replaceAll (#6871) (camc314) - 50744f3 minifier: Implement folding String.prototype.replace (#6870) (camc314) - fccf82e minifier: Implement folding `substring` string fns (#6869) (camc314) - e6a5a1b minifier: Implement folding `charCodeAt` string fns (#6475) (camc314) - 0d0bb17 transformer: Complete the async-to-generator plugin (#6658) (Dunqing) - 419343b traverse: Implement `GetAddress` for `Ancestor` (#6877) (overlookmotel) ### Bug Fixes - a47c70e minifier: Fix remaining runtime bugs (#6855) (Boshen) - 686727f minifier: Reference read has side effect (#6851) (Boshen) - c658d93 minifier: Keep template literals with expressions (#6849) (Boshen) - 4dc5e51 transformer: Only run typescript plugin for typescript source (#6889) (Boshen) - 076f5c3 transformer/typescript: Retain ExportNamedDeclaration without specifiers and declaration (#6848) (Dunqing) - b075982 types: Change @oxc/types package name (#6874) (ottomated) ### Documentation - 6eeb0e6 ast: Mention typescript-eslint, field ordering and shape (#6863) (Boshen) - 99e3b32 napi: Remove JSON.parse from example (#6836) (ottomated) ### Refactor - adb5039 allocator: Add `impl GetAddress for Address` (#6891) (overlookmotel) - 3e7507f ast_tools: Reduce macro usage (#6895) (overlookmotel) - 423d54c rust: Remove the annoying `clippy::wildcard_imports` (#6860) (Boshen) - 2d95009 transformer: Implement `Debug` on `StatementInjector` internal types (#6886) (overlookmotel) - c383c34 transformer: Make `StatementInjectorStore` methods generic over `GetAddress` (#6885) (overlookmotel) - 1f29523 transformer: Rename ReactJsx to Jsx (#6883) (Boshen) - 333b758 transformer: `StatementInjectorStore` methods take `&Statement` as target (#6858) (overlookmotel) - c19996c transformer: Add `StatementInjectorStore::insert_many_before` method (#6857) (overlookmotel) - 7339dde transformer: `StatementInjectorStore::insert_many_after` take an iterator (#6856) (overlookmotel) - 4348eae transformer/typescript: Re-order visitor methods (#6864) (overlookmotel) - 3a56d59 transformer/typescript: Insert assignments after super by `StatementInjector` (#6654) (Dunqing) - a366fae traverse: Rename `TraverseScoping::generate_binding_in_current_scope` (#6832) (overlookmotel) - 3b99fe6 traverse: Move `generate_binding` to `TraverseScoping` (#6831) (overlookmotel) - 60f487a traverse: `TraverseCtx::generate_binding` take an `Atom` (#6830) (overlookmotel) ### Styling - 262b2ed ast: Move crate doc comment to top of file (#6890) (overlookmotel) --------- Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

…#6755) Part of oxc-project#6347. Moves typescript logic from derive_estree into a new ast_tools generator.

## [0.34.0] - 2024-10-26 - 4618aa2 transformer: [**BREAKING**] Rename `TransformerOptions::react` to `jsx` (oxc-project#6888) (Boshen) - 90c786c regular_expression: [**BREAKING**] Support ES2025 Duplicated named capture groups (oxc-project#6847) (leaysgur) - 67a7bde napi/parser: [**BREAKING**] Add typings to napi/parser (oxc-project#6796) (ottomated) ### Features - 1145341 ast_tools: Output typescript to a separate package (oxc-project#6755) (ottomated) - 4429754 ecmascript: Constant eval `null` to number (oxc-project#6879) (Boshen) - fd57e00 ecmascript: Add abstract_relational_comparison to dce (oxc-project#6846) (Boshen) - 8bcaf59 minifier: Late peeophole optimization (oxc-project#6882) (Boshen) - 860cbca minifier: Implement folding simple arrow fns (oxc-project#6875) (camc314) - c26020e minifier: Implement folding String.prototype.replaceAll (oxc-project#6871) (camc314) - 50744f3 minifier: Implement folding String.prototype.replace (oxc-project#6870) (camc314) - fccf82e minifier: Implement folding `substring` string fns (oxc-project#6869) (camc314) - e6a5a1b minifier: Implement folding `charCodeAt` string fns (oxc-project#6475) (camc314) - 0d0bb17 transformer: Complete the async-to-generator plugin (oxc-project#6658) (Dunqing) - 419343b traverse: Implement `GetAddress` for `Ancestor` (oxc-project#6877) (overlookmotel) ### Bug Fixes - a47c70e minifier: Fix remaining runtime bugs (oxc-project#6855) (Boshen) - 686727f minifier: Reference read has side effect (oxc-project#6851) (Boshen) - c658d93 minifier: Keep template literals with expressions (oxc-project#6849) (Boshen) - 4dc5e51 transformer: Only run typescript plugin for typescript source (oxc-project#6889) (Boshen) - 076f5c3 transformer/typescript: Retain ExportNamedDeclaration without specifiers and declaration (oxc-project#6848) (Dunqing) - b075982 types: Change @oxc/types package name (oxc-project#6874) (ottomated) ### Documentation - 6eeb0e6 ast: Mention typescript-eslint, field ordering and shape (oxc-project#6863) (Boshen) - 99e3b32 napi: Remove JSON.parse from example (oxc-project#6836) (ottomated) ### Refactor - adb5039 allocator: Add `impl GetAddress for Address` (oxc-project#6891) (overlookmotel) - 3e7507f ast_tools: Reduce macro usage (oxc-project#6895) (overlookmotel) - 423d54c rust: Remove the annoying `clippy::wildcard_imports` (oxc-project#6860) (Boshen) - 2d95009 transformer: Implement `Debug` on `StatementInjector` internal types (oxc-project#6886) (overlookmotel) - c383c34 transformer: Make `StatementInjectorStore` methods generic over `GetAddress` (oxc-project#6885) (overlookmotel) - 1f29523 transformer: Rename ReactJsx to Jsx (oxc-project#6883) (Boshen) - 333b758 transformer: `StatementInjectorStore` methods take `&Statement` as target (oxc-project#6858) (overlookmotel) - c19996c transformer: Add `StatementInjectorStore::insert_many_before` method (oxc-project#6857) (overlookmotel) - 7339dde transformer: `StatementInjectorStore::insert_many_after` take an iterator (oxc-project#6856) (overlookmotel) - 4348eae transformer/typescript: Re-order visitor methods (oxc-project#6864) (overlookmotel) - 3a56d59 transformer/typescript: Insert assignments after super by `StatementInjector` (oxc-project#6654) (Dunqing) - a366fae traverse: Rename `TraverseScoping::generate_binding_in_current_scope` (oxc-project#6832) (overlookmotel) - 3b99fe6 traverse: Move `generate_binding` to `TraverseScoping` (oxc-project#6831) (overlookmotel) - 60f487a traverse: `TraverseCtx::generate_binding` take an `Atom` (oxc-project#6830) (overlookmotel) ### Styling - 262b2ed ast: Move crate doc comment to top of file (oxc-project#6890) (overlookmotel) --------- Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

github-actions bot added the A-ast-tools Area - AST tools label Oct 21, 2024

This was referenced Oct 21, 2024

fix(wasm): remove type defs for ArrayExpressionElement and Elision #6683

Merged

refactor(ast_tools): give GeneratorOutput and SideEffect named fields #6685

Merged

This was referenced Oct 21, 2024

feat(ast_tools): ability to generate text files #6686

Merged

feat(ast_tools): generate TS type defs as text file #6687

Closed

ottomated marked this pull request as ready for review October 21, 2024 21:33

overlookmotel force-pushed the 10-21-feat_output_typescript_to_a_separate_package branch from 353e0c5 to fda6df0 Compare October 22, 2024 12:10

overlookmotel changed the base branch from 10-19-feat_ast_tools_generate_ts_type_defs_as_text_file to main October 22, 2024 12:10

github-actions bot added A-ast Area - AST labels Oct 22, 2024

overlookmotel self-requested a review October 22, 2024 12:10

overlookmotel changed the title ~~feat: output typescript to a separate package~~ feat(ast_tools): output typescript to a separate package Oct 22, 2024

github-actions bot added the C-enhancement Category - New feature or request label Oct 22, 2024

overlookmotel force-pushed the 10-21-feat_output_typescript_to_a_separate_package branch 2 times, most recently from fda6df0 to a20f980 Compare October 22, 2024 12:16

ottomated marked this pull request as draft October 22, 2024 15:32

ottomated marked this pull request as ready for review October 22, 2024 21:48

ottomated mentioned this pull request Oct 23, 2024

feat(napi/parser)!: add typings to napi/parser #6796

Merged

overlookmotel reviewed Oct 23, 2024

View reviewed changes

tasks/ast_tools/src/generators/typescript.rs Show resolved Hide resolved

tasks/ast_tools/src/generators/typescript.rs Show resolved Hide resolved

tasks/ast_tools/src/generators/typescript.rs Show resolved Hide resolved

overlookmotel requested a review from Boshen October 23, 2024 17:48

Boshen reviewed Oct 24, 2024

View reviewed changes

crates/oxc_ast/src/ast/types.d.ts Outdated Show resolved Hide resolved

overlookmotel approved these changes Oct 24, 2024

View reviewed changes

Boshen added the 0-merge Merge with Graphite Merge Queue label Oct 24, 2024 — with Graphite App

feat(ast_tools): output typescript to a separate package (#6755)

1145341

Part of #6347. Moves typescript logic from derive_estree into a new ast_tools generator.

Boshen force-pushed the 10-21-feat_output_typescript_to_a_separate_package branch from f51b1f5 to 1145341 Compare October 24, 2024 13:09

graphite-app bot merged commit 1145341 into main Oct 24, 2024
27 checks passed

graphite-app bot deleted the 10-21-feat_output_typescript_to_a_separate_package branch October 24, 2024 13:13

This was referenced Oct 24, 2024

Release new @oxc-project/types NPM package #6862

Closed

Implement serde::Serialize on AST types via #[generate_derive] #6347

Open

oxc-bot mentioned this pull request Oct 26, 2024

release(crates): v0.34.0 #6908

Merged

Orenbek pushed a commit to Orenbek/oxc that referenced this pull request Oct 28, 2024

feat(ast_tools): output typescript to a separate package (oxc-project…

d707cbc

…#6755) Part of oxc-project#6347. Moves typescript logic from derive_estree into a new ast_tools generator.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ast_tools): output typescript to a separate package #6755

feat(ast_tools): output typescript to a separate package #6755

ottomated commented Oct 21, 2024 •

edited by overlookmotel

Loading

graphite-app bot commented Oct 21, 2024

ottomated commented Oct 21, 2024 •

edited

Loading

Boshen commented Oct 22, 2024

Boshen commented Oct 22, 2024 •

edited

Loading

codspeed-hq bot commented Oct 22, 2024 •

edited

Loading

overlookmotel commented Oct 22, 2024 •

edited

Loading

ottomated commented Oct 22, 2024

ottomated commented Oct 22, 2024

overlookmotel left a comment •

edited

Loading

overlookmotel commented Oct 23, 2024 •

edited

Loading

Boshen commented Oct 24, 2024

Boshen commented Oct 24, 2024

ottomated commented Oct 24, 2024

overlookmotel left a comment •

edited

Loading

graphite-app bot commented Oct 24, 2024 •

edited

Loading

feat(ast_tools): output typescript to a separate package #6755

feat(ast_tools): output typescript to a separate package #6755

Conversation

ottomated commented Oct 21, 2024 • edited by overlookmotel Loading

graphite-app bot commented Oct 21, 2024

Your org has enabled the Graphite merge queue for merging into main

ottomated commented Oct 21, 2024 • edited Loading

Boshen commented Oct 22, 2024

Boshen commented Oct 22, 2024 • edited Loading

codspeed-hq bot commented Oct 22, 2024 • edited Loading

CodSpeed Performance Report

Merging #6755 will not alter performance

Summary

overlookmotel commented Oct 22, 2024 • edited Loading

Autofix

WASM types

Missing custom types

ottomated commented Oct 22, 2024

ottomated commented Oct 22, 2024

overlookmotel left a comment • edited Loading

Choose a reason for hiding this comment

overlookmotel commented Oct 23, 2024 • edited Loading

Boshen commented Oct 24, 2024

Boshen commented Oct 24, 2024

ottomated commented Oct 24, 2024

overlookmotel left a comment • edited Loading

Choose a reason for hiding this comment

graphite-app bot commented Oct 24, 2024 • edited Loading

Merge activity

ottomated commented Oct 21, 2024 •

edited by overlookmotel

Loading

ottomated commented Oct 21, 2024 •

edited

Loading

Boshen commented Oct 22, 2024 •

edited

Loading

codspeed-hq bot commented Oct 22, 2024 •

edited

Loading

overlookmotel commented Oct 22, 2024 •

edited

Loading

overlookmotel left a comment •

edited

Loading

overlookmotel commented Oct 23, 2024 •

edited

Loading

overlookmotel left a comment •

edited

Loading

graphite-app bot commented Oct 24, 2024 •

edited

Loading