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

Merged
merged 1 commit into from
Feb 3, 2025
Merged

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

merged 1 commit into from
Feb 3, 2025
+162 −51

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. Now with #8839, it works well for typescript as well.

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 (e623745) with main (84b62c7)

Summary

✅ 33 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.
  • Feb 3, 2:20 AM EST: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Feb 3, 2:20 AM EST: A user added this pull request to the Graphite merge queue.
  • Feb 3, 2:28 AM EST: A user merged this pull request with the Graphite merge queue.

@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 2540d75 to a252a7a Compare January 19, 2025 10:54
sapphi-red added a commit that referenced this pull request Jan 19, 2025
@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.

sapphi-red added a commit that referenced this pull request Jan 26, 2025
@sapphi-red
feat(minifier): minify String::concat into template literal (#8443)
9b297fe 
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 044b0ff to 9b297fe Compare January 26, 2025 10:12
sapphi-red added a commit that referenced this pull request Feb 2, 2025
@sapphi-red
feat(minifier): minify String::concat into template literal (#8443)
cfcac66 
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 9b297fe to cfcac66 Compare February 2, 2025 10:12
@sapphi-red sapphi-red changed the base branch from main to 02-02-feat_codegen_prefer_backquotes_over_double___single_quotes February 2, 2025 10:12
@sapphi-red sapphi-red mentioned this pull request Feb 2, 2025
Merged
@sapphi-red sapphi-red marked this pull request as ready for review February 2, 2025 10:19
@sapphi-red sapphi-red force-pushed the 02-02-feat_codegen_prefer_backquotes_over_double___single_quotes branch from 7700322 to df82694 Compare February 3, 2025 06:17
sapphi-red added a commit that referenced this pull request Feb 3, 2025
@sapphi-red
feat(minifier): minify String::concat into template literal (#8443)
f2497d2 
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 cfcac66 to f2497d2 Compare February 3, 2025 06:17
@github-actions github-actions bot added the A-codegen Area - Code Generation label Feb 3, 2025
@sapphi-red sapphi-red force-pushed the 02-02-feat_codegen_prefer_backquotes_over_double___single_quotes branch from df82694 to 3cf8a9c Compare February 3, 2025 06:24
sapphi-red added a commit that referenced this pull request Feb 3, 2025
@sapphi-red
feat(minifier): minify String::concat into template literal (#8443)
96d0189 
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 daec006 to 96d0189 Compare February 3, 2025 06:24
sapphi-red added a commit that referenced this pull request Feb 3, 2025
@sapphi-red
feat(minifier): minify String::concat into template literal (#8443)
bd98584 
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 96d0189 to bd98584 Compare February 3, 2025 06:34
sapphi-red added a commit that referenced this pull request Feb 3, 2025
@sapphi-red
feat(minifier): minify String::concat into template literal (#8443)
6f58dfb 
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 bd98584 to 6f58dfb Compare February 3, 2025 06:37
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Feb 3, 2025
@sapphi-red
feat(minifier): minify String::concat into template literal (#8443)
e623745 
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.~~ Now with #8839, it works well for typescript as well.

**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)
@graphite-app graphite-app bot force-pushed the 02-02-feat_codegen_prefer_backquotes_over_double___single_quotes branch from 3cf8a9c to b4ee617 Compare February 3, 2025 07:21
graphite-app bot pushed a commit that referenced this pull request Feb 3, 2025
@sapphi-red
feat(codegen): prefer backquotes over double / single quotes (#8839)
b4ee617 
When thinking about why #8443 made the gzip size increased, I thought it maybe because the backquotes appear more than before the PR. So I changed the codegen to prefer backquotes than double/single quotes. It seems this separate change have a positive effect with some projects.
My guess is that in most projects, back quotes does not appear in string literals than double quotes and single quotes.

The gzipped size slightly increases for `bundle.min.js`, `victory.js`, `antd.js`, but those will be decreased by #8443 except for `bundle.min.js`.
@graphite-app graphite-app bot force-pushed the 01-11-feat_minifier_minify_string_concat_into_template_literal branch from 6f58dfb to e623745 Compare February 3, 2025 07:21
Base automatically changed from 02-02-feat_codegen_prefer_backquotes_over_double___single_quotes to main February 3, 2025 07:25
@graphite-app graphite-app bot merged commit e623745 into main Feb 3, 2025
26 checks passed
@graphite-app graphite-app bot deleted the 01-11-feat_minifier_minify_string_concat_into_template_literal branch February 3, 2025 07:28
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
0-merge Merge with Graphite Merge Queue A-codegen Area - Code Generation 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