Skip to content

feat(ast)!: add scope_id to TSFunctionType#9559

Merged
graphite-app[bot] merged 1 commit intomainfrom
c/03-05-feat_ast_add_scope_id_to_tsfunctiontype_
Mar 5, 2025
Merged

feat(ast)!: add scope_id to TSFunctionType#9559
graphite-app[bot] merged 1 commit intomainfrom
c/03-05-feat_ast_add_scope_id_to_tsfunctiontype_

Conversation

@camc314
Copy link
Copy Markdown
Contributor

@camc314 camc314 commented Mar 5, 2025

fixes #9558

@github-actions github-actions bot added A-ast Area - AST A-isolated-declarations Isolated Declarations C-enhancement Category - New feature or request labels Mar 5, 2025
Copy link
Copy Markdown
Contributor Author

camc314 commented Mar 5, 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.

@camc314 camc314 marked this pull request as ready for review March 5, 2025 11:08
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 5, 2025

CodSpeed Performance Report

Merging #9559 will create unknown performance changes

Comparing c/03-05-feat_ast_add_scope_id_to_tsfunctiontype_ (48a0394) with main (bbb4f98)

Summary

🆕 39 new benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
🆕 parser_napi[RadixUIAdoptionSection.jsx] N/A 4.8 ms N/A
🆕 parser_napi[pdf.mjs] N/A 1.8 s N/A
🆕 parser_napi_raw[RadixUIAdoptionSection.jsx] N/A 1.3 ms N/A
🆕 parser_napi_raw[cal.com.tsx] N/A 699.1 ms N/A
🆕 parser_napi_raw[checker.ts] N/A 1.1 s N/A
🆕 parser_napi_raw[pdf.mjs] N/A 266.3 ms N/A
🆕 codegen[checker.ts] N/A 23.1 ms N/A
🆕 codegen_sourcemap[checker.ts] N/A 66.1 ms N/A
🆕 isolated-declarations[vue-id.ts] N/A 58.7 ms N/A
🆕 lexer[RadixUIAdoptionSection.jsx] N/A 21 µs N/A
🆕 lexer[antd.js] N/A 24.1 ms N/A
🆕 lexer[cal.com.tsx] N/A 5.7 ms N/A
🆕 lexer[checker.ts] N/A 14.5 ms N/A
🆕 lexer[pdf.mjs] N/A 3.8 ms N/A
🆕 linter[RadixUIAdoptionSection.jsx] N/A 2.7 ms N/A
🆕 linter[cal.com.tsx] N/A 1.2 s N/A
🆕 linter[checker.ts] N/A 2.9 s N/A
🆕 mangler[antd.js] N/A 16.1 ms N/A
🆕 mangler[react.development.js] N/A 294.9 µs N/A
🆕 mangler[typescript.js] N/A 39.7 ms N/A
... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

@camc314 camc314 marked this pull request as draft March 5, 2025 11:19
@Dunqing
Copy link
Copy Markdown
Member

Dunqing commented Mar 5, 2025

Copy link
Copy Markdown
Member

@Dunqing Dunqing left a comment

Choose a reason for hiding this comment

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

I found out how TypeScript handles infer X. The binding of infer should be always inserted into the TSConditionalType scope. So we need to do some check for TSInferType's TSTypeParameter in

fn bind(&self, builder: &mut SemanticBuilder) {
let symbol_id = builder.declare_symbol(
self.name.span,
&self.name.name,
SymbolFlags::TypeParameter,
SymbolFlags::TypeParameterExcludes,
);
self.name.symbol_id.set(Some(symbol_id));
}

This should be changed first in another PR.

@camc314
Copy link
Copy Markdown
Contributor Author

camc314 commented Mar 5, 2025

yes, 🙂 i'm working on this in a bit. Copying typescript-eslint's implementation

@camc314 camc314 force-pushed the c/03-05-feat_ast_add_scope_id_to_tsfunctiontype_ branch 2 times, most recently from 042c41c to 0b320fb Compare March 5, 2025 12:33
@github-actions github-actions bot added the A-semantic Area - Semantic label Mar 5, 2025
@camc314 camc314 changed the title feat(ast)!: add scope_id to TSFunctionType feat(ast,semantic)!: add scope_id to TSFunctionType Mar 5, 2025
@camc314 camc314 force-pushed the c/03-05-feat_ast_add_scope_id_to_tsfunctiontype_ branch from 0b320fb to 8e8b0f6 Compare March 5, 2025 12:36
@github-actions github-actions bot added the A-transformer Area - Transformer / Transpiler label Mar 5, 2025
@camc314 camc314 changed the title feat(ast,semantic)!: add scope_id to TSFunctionType fix(semantic): add scope flags for TS type nodes Mar 5, 2025
@github-actions github-actions bot added the C-bug Category - Bug label Mar 5, 2025
@camc314 camc314 changed the title fix(semantic): add scope flags for TS type nodes feat(ast,semantic)!: add scope_id to TSFunctionType Mar 5, 2025
@camc314 camc314 marked this pull request as ready for review March 5, 2025 12:39
@camc314 camc314 requested a review from Boshen as a code owner March 5, 2025 12:39
@camc314 camc314 requested a review from Dunqing March 5, 2025 12:39
@camc314
Copy link
Copy Markdown
Contributor Author

camc314 commented Mar 5, 2025

@Dunqing please take a look when you've time 🙏 should be correct now

@camc314 camc314 force-pushed the c/03-05-feat_ast_add_scope_id_to_tsfunctiontype_ branch 2 times, most recently from 631c84f to c7147dc Compare March 5, 2025 14:22
@camc314 camc314 force-pushed the c/fix-infer-scope branch from 9c25885 to 42d6dec Compare March 5, 2025 15:16
@camc314 camc314 changed the base branch from c/fix-infer-scope to graphite-base/9559 March 5, 2025 15:16
@camc314 camc314 force-pushed the c/03-05-feat_ast_add_scope_id_to_tsfunctiontype_ branch from bed73ce to e861c6b Compare March 5, 2025 15:19
@camc314 camc314 changed the base branch from graphite-base/9559 to c/fix-infer-scope March 5, 2025 15:19
@camc314 camc314 force-pushed the c/03-05-feat_ast_add_scope_id_to_tsfunctiontype_ branch from e861c6b to 1df2bd1 Compare March 5, 2025 15:23
@camc314 camc314 force-pushed the c/fix-infer-scope branch from c99e687 to e9f1f62 Compare March 5, 2025 15:23
@camc314 camc314 force-pushed the c/03-05-feat_ast_add_scope_id_to_tsfunctiontype_ branch 3 times, most recently from a09b7e5 to c89072e Compare March 5, 2025 15:45
@Dunqing
Copy link
Copy Markdown
Member

Dunqing commented Mar 5, 2025

@camc314 camc314 force-pushed the c/fix-infer-scope branch from e9f1f62 to 491d294 Compare March 5, 2025 16:41
@camc314 camc314 force-pushed the c/03-05-feat_ast_add_scope_id_to_tsfunctiontype_ branch from c89072e to 4e178f9 Compare March 5, 2025 16:41
@camc314 camc314 force-pushed the c/fix-infer-scope branch from 491d294 to 47d7849 Compare March 5, 2025 16:49
@camc314 camc314 force-pushed the c/03-05-feat_ast_add_scope_id_to_tsfunctiontype_ branch 2 times, most recently from feecb6b to 539aa28 Compare March 5, 2025 16:50
@camc314 camc314 force-pushed the c/fix-infer-scope branch from 47d7849 to e94fe89 Compare March 5, 2025 16:50
@Dunqing Dunqing changed the title feat(ast,semantic)!: add scope_id to TSFunctionType feat(ast)!: add scope_id to TSFunctionType Mar 5, 2025
@camc314 camc314 force-pushed the c/fix-infer-scope branch from e94fe89 to a5a10b2 Compare March 5, 2025 16:57
@camc314 camc314 force-pushed the c/03-05-feat_ast_add_scope_id_to_tsfunctiontype_ branch from 539aa28 to 603a94a Compare March 5, 2025 16:57
@Dunqing Dunqing added the 0-merge Merge with Graphite Merge Queue label Mar 5, 2025
Copy link
Copy Markdown
Member

Dunqing commented Mar 5, 2025

Merge activity

  • Mar 5, 12:00 PM EST: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Mar 5, 12:03 PM EST: A user added this pull request to the Graphite merge queue.
  • Mar 5, 12:12 PM EST: A user merged this pull request with the Graphite merge queue.

@graphite-app graphite-app bot force-pushed the c/fix-infer-scope branch from a5a10b2 to bbb4f98 Compare March 5, 2025 17:03
@graphite-app graphite-app bot force-pushed the c/03-05-feat_ast_add_scope_id_to_tsfunctiontype_ branch from 603a94a to 48a0394 Compare March 5, 2025 17:04
Base automatically changed from c/fix-infer-scope to main March 5, 2025 17:08
@graphite-app graphite-app bot merged commit 48a0394 into main Mar 5, 2025
35 checks passed
@graphite-app graphite-app bot deleted the c/03-05-feat_ast_add_scope_id_to_tsfunctiontype_ branch March 5, 2025 17:12
@camc314
Copy link
Copy Markdown
Contributor Author

camc314 commented Mar 5, 2025

thank you @Dunqing for your detailed reviews! 🙏

@oxc-bot oxc-bot mentioned this pull request Mar 6, 2025
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-isolated-declarations Isolated Declarations A-semantic Area - Semantic A-transformer Area - Transformer / Transpiler C-bug Category - Bug C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

isolated declaration transform drops used imports

2 participants