Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(minifier): minify String::concat into template literal #8443

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat(minifier): minify String::concat into template literal #8443

wants to merge 1 commit into from
+174 −55

Conversation

sapphi-red
Copy link
Contributor

@sapphi-red sapphi-red commented Jan 12, 2025 •
edited
Loading

Compress "".concat(a, "b", c) into `${a}b${c}`.

I'm not sure if this should be merged. It works for antd but it doesn't for typescript.

References

This was referenced Jan 12, 2025
Merged
Merged
@sapphi-red Graphite App
Copy link
Contributor Author

sapphi-red commented Jan 12, 2025 •
edited
Loading

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.

@sapphi-red sapphi-red mentioned this pull request Jan 12, 2025
Merged
@github-actions github-actions bot added A-minifier Area - Minifier C-enhancement Category - New feature or request labels Jan 12, 2025
@sapphi-red sapphi-red force-pushed the 01-11-feat_minifier_fold_array_concat_into_literal branch from 6ab26ad to 0ab0835 Compare January 12, 2025 08:56
@sapphi-red sapphi-red force-pushed the 01-11-feat_minifier_minify_string_concat_into_template_literal branch from db5beeb to 72dc0d1 Compare January 12, 2025 08:56
@codspeed-hq CodSpeed HQ
Copy link

codspeed-hq bot commented Jan 12, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #8443 will not alter performance

Comparing 01-11-feat_minifier_minify_string_concat_into_template_literal (044b0ff) with main (d966e0a)

Summary

✅ 32 untouched benchmarks

@sapphi-red sapphi-red force-pushed the 01-11-feat_minifier_fold_array_concat_into_literal branch from 0ab0835 to a031cb7 Compare January 17, 2025 02:52
@sapphi-red sapphi-red force-pushed the 01-11-feat_minifier_minify_string_concat_into_template_literal branch from 72dc0d1 to 0a5a1c2 Compare January 17, 2025 02:53
@sapphi-red sapphi-red force-pushed the 01-11-feat_minifier_fold_array_concat_into_literal branch from a031cb7 to a024338 Compare January 17, 2025 02:59
@sapphi-red sapphi-red force-pushed the 01-11-feat_minifier_minify_string_concat_into_template_literal branch from 0a5a1c2 to fb985ab Compare January 17, 2025 03:00
@sapphi-red sapphi-red marked this pull request as ready for review January 17, 2025 03:06
@sapphi-red sapphi-red force-pushed the 01-11-feat_minifier_fold_array_concat_into_literal branch from a024338 to 9900d1e Compare January 17, 2025 03:26
@sapphi-red sapphi-red force-pushed the 01-11-feat_minifier_minify_string_concat_into_template_literal branch from fb985ab to d6df4f8 Compare January 17, 2025 03:26
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Jan 17, 2025
@Boshen Graphite App
Copy link
Member

Boshen commented Jan 17, 2025 •
edited by graphite-app bot
Loading

Merge activity

  • Jan 17, 12:53 AM EST: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jan 17, 12:53 AM EST: A user added this pull request to the Graphite merge queue.
  • Jan 17, 1:14 AM EST: The Graphite merge queue couldn't merge this PR because it was in draft mode.

@Boshen Boshen marked this pull request as draft January 17, 2025 05:53
@Boshen
Copy link
Member

Boshen commented Jan 17, 2025

but it doesn't for typescript.

Then it shouldn't 😅 what's broken?

@Boshen Boshen force-pushed the 01-11-feat_minifier_fold_array_concat_into_literal branch from 9900d1e to df9b2d1 Compare January 17, 2025 05:55
Boshen pushed a commit that referenced this pull request Jan 17, 2025
@sapphi-red
feat(minifier): minify String::concat into template literal (#8443)
f928cb5 
Compress `"".concat(a, "b", c)` into `` `${a}b${c}` ``.

I'm not sure if this should be merged. It works for antd but it doesn't for typescript.

**References**
- [Spec of `String::concat`](https://tc39.es/ecma262/multipage/text-processing.html#sec-string.prototype.concat)
- [Spec of template literal](https://tc39.es/ecma262/multipage/ecmascript-language-expressions.html#sec-template-literals-runtime-semantics-evaluation)
@Boshen Boshen force-pushed the 01-11-feat_minifier_minify_string_concat_into_template_literal branch from d6df4f8 to f928cb5 Compare January 17, 2025 05:56
@sapphi-red
Copy link
Contributor Author

but it doesn't for typescript.

Then it shouldn't 😅 what's broken?

Ah, I just mean it makes the output larger when gziped. It's not broken.

  • antd.js: 492.44 kB -> 492.20 kB
  • typescript.js: 907.09 kB -> 907.28 kB

@Boshen Boshen force-pushed the 01-11-feat_minifier_fold_array_concat_into_literal branch from df9b2d1 to 991a22f Compare January 17, 2025 06:00
Boshen pushed a commit that referenced this pull request Jan 17, 2025
@sapphi-red
feat(minifier): minify String::concat into template literal (#8443)
917a5e7 
Compress `"".concat(a, "b", c)` into `` `${a}b${c}` ``.

I'm not sure if this should be merged. It works for antd but it doesn't for typescript.

**References**
- [Spec of `String::concat`](https://tc39.es/ecma262/multipage/text-processing.html#sec-string.prototype.concat)
- [Spec of template literal](https://tc39.es/ecma262/multipage/ecmascript-language-expressions.html#sec-template-literals-runtime-semantics-evaluation)
@Boshen Boshen force-pushed the 01-11-feat_minifier_minify_string_concat_into_template_literal branch from f928cb5 to 917a5e7 Compare January 17, 2025 06:01
Base automatically changed from 01-11-feat_minifier_fold_array_concat_into_literal to main January 17, 2025 06:13
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Jan 17, 2025
sapphi-red added a commit that referenced this pull request Jan 17, 2025
@sapphi-red
feat(minifier): minify String::concat into template literal (#8443)
6e1122d 
Compress `"".concat(a, "b", c)` into `` `${a}b${c}` ``.

I'm not sure if this should be merged. It works for antd but it doesn't for typescript.

**References**
- [Spec of `String::concat`](https://tc39.es/ecma262/multipage/text-processing.html#sec-string.prototype.concat)
- [Spec of template literal](https://tc39.es/ecma262/multipage/ecmascript-language-expressions.html#sec-template-literals-runtime-semantics-evaluation)
@sapphi-red sapphi-red force-pushed the 01-11-feat_minifier_minify_string_concat_into_template_literal branch from 917a5e7 to 6e1122d Compare January 17, 2025 07:27
sapphi-red added a commit that referenced this pull request Jan 19, 2025
@sapphi-red
feat(minifier): minify String::concat into template literal (#8443)
8e33ea8 
Compress `"".concat(a, "b", c)` into `` `${a}b${c}` ``.

I'm not sure if this should be merged. It works for antd but it doesn't for typescript.

**References**
- [Spec of `String::concat`](https://tc39.es/ecma262/multipage/text-processing.html#sec-string.prototype.concat)
- [Spec of template literal](https://tc39.es/ecma262/multipage/ecmascript-language-expressions.html#sec-template-literals-runtime-semantics-evaluation)
@sapphi-red sapphi-red force-pushed the 01-11-feat_minifier_minify_string_concat_into_template_literal branch from 6e1122d to 8e33ea8 Compare January 19, 2025 10:24
sapphi-red added a commit that referenced this pull request Jan 19, 2025
@sapphi-red
feat(minifier): minify String::concat into template literal (#8443)
2540d75 
Compress `"".concat(a, "b", c)` into `` `${a}b${c}` ``.

I'm not sure if this should be merged. It works for antd but it doesn't for typescript.

**References**
- [Spec of `String::concat`](https://tc39.es/ecma262/multipage/text-processing.html#sec-string.prototype.concat)
- [Spec of template literal](https://tc39.es/ecma262/multipage/ecmascript-language-expressions.html#sec-template-literals-runtime-semantics-evaluation)
@sapphi-red sapphi-red force-pushed the 01-11-feat_minifier_minify_string_concat_into_template_literal branch from 8e33ea8 to 2540d75 Compare January 19, 2025 10:27
sapphi-red added a commit that referenced this pull request Jan 19, 2025
@sapphi-red
feat(minifier): minify String::concat into template literal (#8443)
a252a7a 
Compress `"".concat(a, "b", c)` into `` `${a}b${c}` ``.

I'm not sure if this should be merged. It works for antd but it doesn't for typescript.

**References**
- [Spec of `String::concat`](https://tc39.es/ecma262/multipage/text-processing.html#sec-string.prototype.concat)
- [Spec of template literal](https://tc39.es/ecma262/multipage/ecmascript-language-expressions.html#sec-template-literals-runtime-semantics-evaluation)
@sapphi-red sapphi-red force-pushed the 01-11-feat_minifier_minify_string_concat_into_template_literal branch from 2540d75 to a252a7a Compare January 19, 2025 10:54
@sapphi-red
feat(minifier): minify String::concat into template literal (#8443)
044b0ff 
Compress `"".concat(a, "b", c)` into `` `${a}b${c}` ``.

I'm not sure if this should be merged. It works for antd but it doesn't for typescript.

**References**
- [Spec of `String::concat`](https://tc39.es/ecma262/multipage/text-processing.html#sec-string.prototype.concat)
- [Spec of template literal](https://tc39.es/ecma262/multipage/ecmascript-language-expressions.html#sec-template-literals-runtime-semantics-evaluation)
@sapphi-red sapphi-red force-pushed the 01-11-feat_minifier_minify_string_concat_into_template_literal branch from a252a7a to 044b0ff Compare January 19, 2025 10:56
@sapphi-red
Copy link
Contributor Author

I'm not sure why the gzip works better for TypeScript without this PR. Maybe it's because .concat(a,b,c) shares more similar patten than template literals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
A-minifier Area - Minifier C-enhancement Category - New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sapphi-red @Boshen