Skip to content

Commit

Permalink
feat(es/codegen): Skip whitespaces for comments in minify mode (#6465)
Browse files Browse the repository at this point in the history 
Co-authored-by: Donny/강동윤 <[email protected]>
  • Loading branch information
@alexander-akait @kdy1
alexander-akait and kdy1 authored Nov 20, 2022
1 parent 5a3ae50 commit 08a9e21
Show file tree
Hide file tree
Showing 9 changed files with 96 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .github/workflows/CI.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ jobs:
profile: minimal

- name: Patch
shell: bash
run: |
echo '[patch.crates-io]' >> bindings/Cargo.toml
./scripts/cargo/patch-section.sh >> bindings/Cargo.toml
Expand Down Expand Up @@ -703,6 +704,7 @@ jobs:
${{ !contains(github.event.head_commit.message, 'chore: ') }}
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
os:
- ubuntu-latest
Expand All @@ -723,8 +725,10 @@ jobs:
cache: "yarn"

- name: Patch
shell: bash
run: |
echo '[patch.crates-io]' >> bindings/Cargo.toml
./scripts/cargo/patch-section.sh
./scripts/cargo/patch-section.sh >> bindings/Cargo.toml
cd bindings && cargo update
Expand Down Expand Up @@ -784,6 +788,7 @@ jobs:
run: curl https://rustwasm.github.io/wasm-pack/installer/init.sh -sSf | sh

- name: Patch
shell: bash
run: |
echo '[patch.crates-io]' >> bindings/Cargo.toml
./scripts/cargo/patch-section.sh >> bindings/Cargo.toml
Expand Down
12 changes: 9 additions & 3 deletions crates/swc_ecma_codegen/src/comments.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,21 @@ macro_rules! write_comments {
for cmt in cmts.iter() {
match cmt.kind {
CommentKind::Line => {
if $prefix_space {
if $prefix_space && !$e.cfg.minify {
$e.wr.write_comment(" ")?;
}

srcmap!($e, cmt, true);

$e.wr.write_comment("//")?;
$e.wr.write_comment(&cmt.text)?;

srcmap!($e, cmt, false);

$e.wr.write_line()?;
}
CommentKind::Block => {
if $prefix_space {
if $prefix_space && !$e.cfg.minify {
$e.wr.write_comment(" ")?;
}

Expand All @@ -39,7 +43,9 @@ macro_rules! write_comments {
}
$e.wr.write_comment("*/")?;

$e.wr.write_space()?;
if !$e.cfg.minify {
$e.wr.write_space()?;
}
}
}
}
Expand Down
36 changes: 36 additions & 0 deletions crates/swc_html_minifier/tests/fixture/element/script-custom-type/input.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport"
content="width=device-width, user-scalable=no, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0">
<title>Document</title>
</head>
<body>
<script type="text/ng-template">
<!--test-->
<div>
<span> foobar </span>
</div>
</script>

<script type="text/ng-template">
<!--test-->
<div>
<span> foobar </span>
</div>
</script>
<script type="text/html">
<!--test-->
<div>
<span> foobar </span>
</div>
</script>
<script type="text/html">
<!--test-->
<div>
<span> foobar </span>
</div>
</script>
</body>
</html>
21 changes: 21 additions & 0 deletions crates/swc_html_minifier/tests/fixture/element/script-custom-type/output.min.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<!doctype html><html lang=en><meta charset=UTF-8><meta name=viewport content="width=device-width,user-scalable=no,initial-scale=1.0,maximum-scale=1.0,minimum-scale=1.0"><title>Document</title><body><script type=text/ng-template>
<!--test-->
<div>
<span> foobar </span>
</div>
</script><script type=text/ng-template>
<!--test-->
<div>
<span> foobar </span>
</div>
</script><script type=text/html>
<!--test-->
<div>
<span> foobar </span>
</div>
</script><script type=text/html>
<!--test-->
<div>
<span> foobar </span>
</div>
</script>
4 changes: 2 additions & 2 deletions crates/swc_html_minifier/tests/fixture/element/script-options-2/output.min.html
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<!doctype html><html lang=en><title>Document</title><div>Script:</div>
<script>/* Should mangle top level stuff */ var o=function(){let o="bar";o&&(o+="baz"),console.log(o)}</script>
<script>/* Should mangle top level stuff */var o=function(){let o="bar";o&&(o+="baz"),console.log(o)}</script>

<div>Module:</div>
<script type=module>/* Should mangle top level stuff */ var o=function(){let o="bar";o&&(o+="baz"),console.log(o)}</script>
<script type=module>/* Should mangle top level stuff */var o=function(){let o="bar";o&&(o+="baz"),console.log(o)}</script>
11 changes: 11 additions & 0 deletions crates/swc_html_minifier/tests/fixture/element/script-options/input.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,16 @@
// Keep comment
console.log("test " + "😀" + " test");
</script>
<script>
// Comment
var a = "test";
/* Comment */
var b = "test";
var /* Comment */ c /* Comment */ = /* Comment */ "test" /* Comment */;
</script>
<script>
var a = "test" // test
;
</script>
</body>
</html>
6 changes: 4 additions & 2 deletions crates/swc_html_minifier/tests/fixture/element/script-options/output.min.html
View file
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
<!doctype html><html lang=en><title>Document</title><body><script type=module>/* Keep comment */ debugger;// Keep comment
console.log("test \u{1F600} test")</script>
<!doctype html><html lang=en><title>Document</title><body><script type=module>/* Keep comment */debugger;// Keep comment
console.log("test \u{1F600} test")</script><script>// Comment
var a="test",b="test",/* Comment */c/* Comment */=/* Comment */"test"/* Comment */,a="test"// test
</script>
8 changes: 4 additions & 4 deletions node-swc/__tests__/minify_test.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ describe("should remove comments", () => {
);
});

it("should preserve licnese", async () => {
it("should preserve license", async () => {
const { code } = await swc.minify(
`
(function(){
Expand All @@ -213,10 +213,10 @@ describe("should remove comments", () => {
expect(code).toMatchInlineSnapshot(`
"(function(){/**
* @license
*/ const o=Math.random()+\\"_\\"+Math.random();console.log(o)})();"
*/const o=Math.random()+\\"_\\"+Math.random();console.log(o)})();"
`);
});
it("should remove comment near to licnese", async () => {
it("should remove comment near to license", async () => {
const { code } = await swc.minify(
`
(function(){
Expand All @@ -241,7 +241,7 @@ describe("should remove comments", () => {
expect(code).toMatchInlineSnapshot(`
"(function(){/**
* @license
*/ const o=Math.random()+\\"_\\"+Math.random();console.log(o)})();"
*/const o=Math.random()+\\"_\\"+Math.random();console.log(o)})();"
`);
});
});
6 changes: 4 additions & 2 deletions scripts/cargo/patch-section.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ set -eu
SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )

function toLine {
arr=(${1//\\\t/ })
# >&2 echo "toLine: $@"
arr=(${1//,/ })
# >&2 echo "arr: ${arr[0]} ${arr[1]}"
dir="$(dirname ${arr[1]})"
echo "${arr[0]} = { path = '$dir' }"
}

export -f toLine

$SCRIPT_DIR/list-crates.sh | jq '[.name, .manifest_path] | @tsv' | xargs -L 1 -I {} bash -c 'toLine "$@"' _ {}
$SCRIPT_DIR/list-crates.sh | jq '[.name, .manifest_path] | @csv' -r | xargs -I {} bash -c 'toLine "$@"' _ {}

1 comment on commit 08a9e21

@github-actions
Copy link

Choose a reason for hiding this comment

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

Benchmark

Benchmark suite Current: 08a9e21 Previous: 9154bbc Ratio
es/full/bugs-1 343924 ns/iter (± 22485) 389841 ns/iter (± 22054) 0.88
es/full/minify/libraries/antd 1843749333 ns/iter (± 35421072) 2094374257 ns/iter (± 82448426) 0.88
es/full/minify/libraries/d3 387498447 ns/iter (± 5737899) 502630299 ns/iter (± 27607030) 0.77
es/full/minify/libraries/echarts 1572904733 ns/iter (± 20004804) 1873070557 ns/iter (± 758763072) 0.84
es/full/minify/libraries/jquery 98456298 ns/iter (± 1250161) 125668872 ns/iter (± 7586107) 0.78
es/full/minify/libraries/lodash 118166720 ns/iter (± 2299627) 161579804 ns/iter (± 21023829) 0.73
es/full/minify/libraries/moment 59148603 ns/iter (± 1227695) 88708545 ns/iter (± 29765105) 0.67
es/full/minify/libraries/react 19805763 ns/iter (± 491219) 26280308 ns/iter (± 4046363) 0.75
es/full/minify/libraries/terser 294796142 ns/iter (± 8290835) 374533985 ns/iter (± 18064188) 0.79
es/full/minify/libraries/three 565727456 ns/iter (± 7292018) 652642002 ns/iter (± 31323109) 0.87
es/full/minify/libraries/typescript 3371443301 ns/iter (± 25883126) 4136959214 ns/iter (± 252717032) 0.81
es/full/minify/libraries/victory 813563327 ns/iter (± 9071427) 927628403 ns/iter (± 61741347) 0.88
es/full/minify/libraries/vue 149708038 ns/iter (± 2072901) 178428798 ns/iter (± 9735215) 0.84
es/full/codegen/es3 32100 ns/iter (± 588) 35629 ns/iter (± 8609) 0.90
es/full/codegen/es5 32174 ns/iter (± 943) 38103 ns/iter (± 6502) 0.84
es/full/codegen/es2015 32209 ns/iter (± 599) 36890 ns/iter (± 5849) 0.87
es/full/codegen/es2016 32185 ns/iter (± 691) 34100 ns/iter (± 6093) 0.94
es/full/codegen/es2017 32113 ns/iter (± 1559) 38119 ns/iter (± 7737) 0.84
es/full/codegen/es2018 32131 ns/iter (± 932) 37150 ns/iter (± 6840) 0.86
es/full/codegen/es2019 32293 ns/iter (± 1393) 36291 ns/iter (± 6934) 0.89
es/full/codegen/es2020 32442 ns/iter (± 1290) 36868 ns/iter (± 5595) 0.88
es/full/all/es3 186157363 ns/iter (± 4084748) 225387040 ns/iter (± 24353389) 0.83
es/full/all/es5 176933693 ns/iter (± 3280462) 213631035 ns/iter (± 17082855) 0.83
es/full/all/es2015 141729882 ns/iter (± 9827151) 169123795 ns/iter (± 9833735) 0.84
es/full/all/es2016 140943158 ns/iter (± 2932572) 171026993 ns/iter (± 15511849) 0.82
es/full/all/es2017 140890007 ns/iter (± 7364971) 169572869 ns/iter (± 12352684) 0.83
es/full/all/es2018 140345275 ns/iter (± 8031443) 164007453 ns/iter (± 10050642) 0.86
es/full/all/es2019 138207241 ns/iter (± 4187062) 164051983 ns/iter (± 11626394) 0.84
es/full/all/es2020 132581196 ns/iter (± 2728081) 161819590 ns/iter (± 14091917) 0.82
es/full/parser 693874 ns/iter (± 30441) 822178 ns/iter (± 111880) 0.84
es/full/base/fixer 25086 ns/iter (± 896) 28915 ns/iter (± 4716) 0.87
es/full/base/resolver_and_hygiene 88295 ns/iter (± 6324) 97233 ns/iter (± 11134) 0.91
serialization of ast node 212 ns/iter (± 9) 244 ns/iter (± 32) 0.87
serialization of serde 217 ns/iter (± 7) 228 ns/iter (± 28) 0.95

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.