Skip to content

feat(transformer): add /* @_PURE__ */ annotation for pure calls#9424

Merged
graphite-app[bot] merged 1 commit intomainfrom
02-28-feat_transformer_add____pure_____annotation_for_pure_functions
Mar 4, 2025
Merged

feat(transformer): add /* @_PURE__ */ annotation for pure calls#9424
graphite-app[bot] merged 1 commit intomainfrom
02-28-feat_transformer_add____pure_____annotation_for_pure_functions

Conversation

@Dunqing
Copy link
Member

@Dunqing Dunqing commented Feb 28, 2025

close: #6072

Many tests outputs are updated because we need to print /* @_PURE__ */ for the pure calls.

@github-actions github-actions bot added A-transformer Area - Transformer / Transpiler C-enhancement Category - New feature or request labels Feb 28, 2025
Copy link
Member Author

Dunqing commented Feb 28, 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.

@Dunqing Dunqing force-pushed the 02-28-feat_transformer_add____pure_____annotation_for_pure_functions branch from 9895df1 to da3b1a8 Compare February 28, 2025 03:52
@Boshen
Copy link
Member

Boshen commented Feb 28, 2025

It seems like we need to turn off comments in conformance codegen.

@codspeed-hq
Copy link

codspeed-hq bot commented Feb 28, 2025

CodSpeed Performance Report

Merging #9424 will create unknown performance changes

Comparing 02-28-feat_transformer_add____pure_____annotation_for_pure_functions (45ace44) with main (59a9f1d)

Summary

🆕 33 new benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
🆕 codegen[checker.ts] N/A 23.1 ms N/A
🆕 codegen_sourcemap[checker.ts] N/A 66.2 ms N/A
🆕 isolated-declarations[vue-id.ts] N/A 57.9 ms N/A
🆕 lexer[RadixUIAdoptionSection.jsx] N/A 20.8 µ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 297.7 µs N/A
🆕 mangler[typescript.js] N/A 39.8 ms N/A
🆕 minifier[antd.js] N/A 163 ms N/A
🆕 minifier[react.development.js] N/A 1.8 ms N/A
🆕 minifier[typescript.js] N/A 285.8 ms N/A
🆕 estree[checker.ts] N/A 91.8 ms N/A
🆕 parser[RadixUIAdoptionSection.jsx] N/A 88.9 µs N/A
🆕 parser[antd.js] N/A 111.9 ms N/A
... ... ... ... ...

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

@Dunqing
Copy link
Member Author

Dunqing commented Feb 28, 2025

It seems like we need to turn off comments in conformance codegen.

You mean turn off pure annotation comments? We need this to test if we added pure correctly. I found a lot of tests failed due to the way it is printed is not the same.

For example:

 var HelloMessage = React.createClass({ render: function() {
   return /* @__PURE__ */ React.createElement("div", null, "Hello ", this.props.name);
 } });
-React.render(
-  /* @__PURE__ */ React.createElement(HelloMessage, { name: /* @__PURE__ */ React.createElement("span", null, "Sebastian") }),
-  mountNode
-);
+React.render(/* @__PURE__ */ React.createElement(HelloMessage, { name: /* @__PURE__ */ React.createElement("span", null, "Sebastian") }), mountNode);

I will dig into this problem later.

@Dunqing Dunqing force-pushed the 02-27-feat_ast_add_astbuilder__with_pure_etc_methods branch from 5c54b9c to 2fa4b60 Compare March 3, 2025 10:27
@Dunqing Dunqing force-pushed the 02-28-feat_transformer_add____pure_____annotation_for_pure_functions branch 2 times, most recently from 5305812 to 643a2e6 Compare March 3, 2025 12:22
@Dunqing Dunqing force-pushed the 02-27-feat_ast_add_astbuilder__with_pure_etc_methods branch from 2fa4b60 to 6cab457 Compare March 3, 2025 16:09
@Dunqing Dunqing force-pushed the 02-28-feat_transformer_add____pure_____annotation_for_pure_functions branch 3 times, most recently from 72b0a1d to 8f53bcd Compare March 4, 2025 01:23
@Dunqing Dunqing force-pushed the 02-27-feat_ast_add_astbuilder__with_pure_etc_methods branch from 6cab457 to 5042261 Compare March 4, 2025 01:23
@Dunqing Dunqing force-pushed the 02-28-feat_transformer_add____pure_____annotation_for_pure_functions branch from 8f53bcd to d2bc1d3 Compare March 4, 2025 01:32
@Dunqing Dunqing marked this pull request as ready for review March 4, 2025 01:52
@Dunqing Dunqing requested a review from overlookmotel as a code owner March 4, 2025 01:52
@Dunqing Dunqing requested a review from Boshen March 4, 2025 01:53
Copy link
Member

@Boshen Boshen left a comment

Choose a reason for hiding this comment

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

Feel free to merge after fixing the conformance issues.

@Dunqing Dunqing force-pushed the 02-28-feat_transformer_add____pure_____annotation_for_pure_functions branch from d2bc1d3 to 7f62265 Compare March 4, 2025 03:00
@Dunqing
Copy link
Member Author

Dunqing commented Mar 4, 2025

Feel free to merge after fixing the conformance issues.

Fixed in #9530

@graphite-app graphite-app bot added the 0-merge Merge with Graphite Merge Queue label Mar 4, 2025
@graphite-app
Copy link
Contributor

graphite-app bot commented Mar 4, 2025

Merge activity

)

close: #6072

Many tests outputs are updated because we need to print `/* @_PURE__ */` for the pure calls.
@graphite-app graphite-app bot force-pushed the 02-27-feat_ast_add_astbuilder__with_pure_etc_methods branch from 5042261 to 59a9f1d Compare March 4, 2025 06:13
@graphite-app graphite-app bot force-pushed the 02-28-feat_transformer_add____pure_____annotation_for_pure_functions branch from 7f62265 to 45ace44 Compare March 4, 2025 06:13
graphite-app bot pushed a commit that referenced this pull request Mar 4, 2025
Base automatically changed from 02-27-feat_ast_add_astbuilder__with_pure_etc_methods to main March 4, 2025 06:18
@graphite-app graphite-app bot merged commit 45ace44 into main Mar 4, 2025
27 checks passed
@graphite-app graphite-app bot deleted the 02-28-feat_transformer_add____pure_____annotation_for_pure_functions branch March 4, 2025 06:23
@oxc-bot oxc-bot mentioned this pull request Mar 5, 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-transformer Area - Transformer / Transpiler C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

codegen, transformer: print pure annotation comments for jsx

2 participants