Skip to content

fix(span/estree): skip ModuleKind::Unambiguous varient for estree#10146

Merged
graphite-app[bot] merged 1 commit intomainfrom
03-31-fix_span_estree_skip_modulekind_unambiguous_varient_for_estree
Apr 3, 2025
Merged

fix(span/estree): skip ModuleKind::Unambiguous varient for estree#10146
graphite-app[bot] merged 1 commit intomainfrom
03-31-fix_span_estree_skip_modulekind_unambiguous_varient_for_estree

Conversation

@Dunqing
Copy link
Member

@Dunqing Dunqing commented Mar 31, 2025

close: #10139

The ModuleKind::Unambiguous can only occur in the parser stage, after being parsed, the ModuleKind certainly changes to ModuleKind::Script or ModuleKind::Module; otherwise is a bug in the parser.

Copy link
Member Author

Dunqing commented Mar 31, 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.

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

@github-actions github-actions bot added the C-bug Category - Bug label Mar 31, 2025
@Dunqing Dunqing changed the title fix(span/estree): skip ModuleKind::Unambiguous varient for estree fix(span/estree): skip ModuleKind::Unambiguous varient for estree Mar 31, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Mar 31, 2025

CodSpeed Instrumentation Performance Report

Merging #10146 will degrade performances by 3.76%

Comparing 03-31-fix_span_estree_skip_modulekind_unambiguous_varient_for_estree (52f2a40) with main (06fc07c)

Summary

⚡ 1 improvements
❌ 1 regressions
✅ 34 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
formatter[antd.js] 7.6 ms 7.9 ms -3.76%
formatter[typescript.js] 7.4 ms 7.2 ms +3.41%

@Dunqing Dunqing force-pushed the 03-31-fix_span_estree_skip_modulekind_unambiguous_varient_for_estree branch from 495313c to c35cd5d Compare March 31, 2025 07:36
@Dunqing Dunqing force-pushed the 03-31-feat_ast_tools_support_estree_skip_for_enum_varient branch from b95efce to c15181f Compare March 31, 2025 07:36
@overlookmotel overlookmotel changed the base branch from 03-31-feat_ast_tools_support_estree_skip_for_enum_varient to graphite-base/10146 April 3, 2025 16:36
@overlookmotel overlookmotel force-pushed the 03-31-fix_span_estree_skip_modulekind_unambiguous_varient_for_estree branch from c35cd5d to 19adb7e Compare April 3, 2025 16:37
@overlookmotel overlookmotel changed the base branch from graphite-base/10146 to 03-31-feat_ast_tools_support_estree_skip_for_enum_varient April 3, 2025 16:37
Copy link
Member

@overlookmotel overlookmotel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I'd prefer it if we had a separate ParserSourceType which is used in parser options, and the SourceType used in AST didn't have the "Unambiguous" variant at all - so you can't create an AST with this erroneous source type which will panic when serialized.

But for now, yes, this seems like the best solution.

@overlookmotel overlookmotel added the 0-merge Merge with Graphite Merge Queue label Apr 3, 2025
Copy link
Member

overlookmotel commented Apr 3, 2025

Merge activity

…#10146)

close: #10139

The `ModuleKind::Unambiguous` can only occur in the parser stage, after being parsed, the `ModuleKind` certainly changes to `ModuleKind::Script` or `ModuleKind::Module`; otherwise is a bug in the parser.
@graphite-app graphite-app bot force-pushed the 03-31-feat_ast_tools_support_estree_skip_for_enum_varient branch from e780138 to 55b7b87 Compare April 3, 2025 16:47
@graphite-app graphite-app bot force-pushed the 03-31-fix_span_estree_skip_modulekind_unambiguous_varient_for_estree branch from 19adb7e to 52f2a40 Compare April 3, 2025 16:47
@overlookmotel
Copy link
Member

Ah it seems I suggested the separate SourceType idea before: #6249.

Base automatically changed from 03-31-feat_ast_tools_support_estree_skip_for_enum_varient to main April 3, 2025 16:53
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Apr 3, 2025
@graphite-app graphite-app bot merged commit 52f2a40 into main Apr 3, 2025
26 checks passed
@graphite-app graphite-app bot deleted the 03-31-fix_span_estree_skip_modulekind_unambiguous_varient_for_estree branch April 3, 2025 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ESTree AST and Oxc AST type discrepancies

2 participants