Skip to content

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Oct 19, 2024

Follow-on after #6404.

ArrayExpressionElement and Elision are not used in the TS types, because ArrayExpression has an override for the field it uses.

pub struct ArrayExpression<'a> {
#[estree(flatten)]
pub span: Span,
#[tsify(type = "Array<SpreadElement | Expression | null>")]
pub elements: Vec<'a, ArrayExpressionElement<'a>>,
/// Array trailing comma
/// <https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Trailing_commas#arrays>
#[estree(skip)]
pub trailing_comma: Option<Span>,
}

Prevent these TS type defs being emitted by introducing a new #[estree(custom_ts_def)] attr, to go with #[estree(custom_serialize)].

@graphite-app
Copy link
Contributor

graphite-app bot commented Oct 19, 2024

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.

Copy link
Member Author

overlookmotel commented Oct 19, 2024

@overlookmotel overlookmotel force-pushed the 10-19-refactor_ast_tools_use_spaces_not_tabs_in_ts_type_defs branch from e1c7f40 to af4c74e Compare October 19, 2024 09:34
@overlookmotel overlookmotel force-pushed the 10-19-fix_wasm_remove_type_def_for_arrayexpressionelement_ branch from 8938d54 to a798ae6 Compare October 19, 2024 09:34
@github-actions github-actions bot added A-ast Area - AST A-ast-tools Area - AST tools C-bug Category - Bug labels Oct 19, 2024
@overlookmotel overlookmotel changed the title fix(wasm): remove type def for ArrayExpressionElement fix(wasm): remove type defs for ArrayExpressionElement and Elision Oct 19, 2024
@overlookmotel overlookmotel marked this pull request as ready for review October 19, 2024 09:42
@overlookmotel overlookmotel force-pushed the 10-19-fix_wasm_remove_type_def_for_arrayexpressionelement_ branch from a798ae6 to 13f22eb Compare October 19, 2024 09:47
@overlookmotel
Copy link
Member Author

@ottomated This is the most substantial change in this stack of PRs. The change is correct, but implementation is maybe a bit hacky. You may see a better way to achieve this.

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 19, 2024

CodSpeed Performance Report

Merging #6683 will not alter performance

Comparing 10-19-fix_wasm_remove_type_def_for_arrayexpressionelement_ (53049fe) with main (d4a2529)

Summary

✅ 30 untouched benchmarks

@overlookmotel overlookmotel force-pushed the 10-19-refactor_ast_tools_use_spaces_not_tabs_in_ts_type_defs branch from af4c74e to e4e1deb Compare October 19, 2024 20:09
@overlookmotel overlookmotel force-pushed the 10-19-fix_wasm_remove_type_def_for_arrayexpressionelement_ branch from 13f22eb to e865bb8 Compare October 19, 2024 20:10
@overlookmotel overlookmotel force-pushed the 10-19-fix_wasm_remove_type_def_for_arrayexpressionelement_ branch from e865bb8 to 0234904 Compare October 20, 2024 20:49
@overlookmotel overlookmotel changed the base branch from 10-19-refactor_ast_tools_use_spaces_not_tabs_in_ts_type_defs to main October 20, 2024 20:49
@overlookmotel overlookmotel added the 0-merge Merge with Graphite Merge Queue label Oct 20, 2024 — with Graphite App
@overlookmotel overlookmotel force-pushed the 10-19-fix_wasm_remove_type_def_for_arrayexpressionelement_ branch from 0234904 to ca8b12c Compare October 20, 2024 21:18
#6683)

Follow-on after #6404.

`ArrayExpressionElement` and `Elision` are not used in the TS types, because `ArrayExpression` has an override for the field it uses.

https://github.com/oxc-project/oxc/blob/002289b4b1b0d6fac080c4b12f4939c8f455272f/crates/oxc_ast/src/ast/js.rs#L293-L302

Prevent these TS type defs being emitted by introducing a new `#[estree(custom_ts_def)]` attr, to go with `#[estree(custom_serialize)]`.
@overlookmotel overlookmotel force-pushed the 10-19-fix_wasm_remove_type_def_for_arrayexpressionelement_ branch from ca8b12c to 53049fe Compare October 20, 2024 21:22
@graphite-app
Copy link
Contributor

graphite-app bot commented Oct 20, 2024

Merge activity

@graphite-app graphite-app bot merged commit 53049fe into main Oct 20, 2024
@graphite-app graphite-app bot deleted the 10-19-fix_wasm_remove_type_def_for_arrayexpressionelement_ branch October 20, 2024 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0-merge Merge with Graphite Merge Queue A-ast Area - AST A-ast-tools Area - AST tools C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants