From d291e81730b9cd6fef9049188a8e7560186b0f2d Mon Sep 17 00:00:00 2001 From: Jakub Pawlowicz Date: Sun, 8 Jan 2017 12:09:50 +0100 Subject: [PATCH] See #807 - disables restructuring optimizations. Can be brought back with: * `{ level: { 2: { restructuring: true } } }` in API; * `-O2 restructuring:on` in CLI. Why: * Those can be slow on big files; * Need further optimizations and code cleanup, see #533. --- History.md | 1 + README.md | 4 ++-- bin/cleancss | 2 +- lib/options/optimization-level.js | 2 +- test/batch-test.js | 14 +++++++++--- test/binary-test.js | 4 ++-- test/integration-test.js | 2 +- test/optimizer/merge-adjacent-test.js | 10 ++++++++- .../merge-non-adjacent-by-body-test.js | 2 +- test/optimizer/reduce-non-adjacent-test.js | 22 +++++++++++++------ test/optimizer/restructure-test.js | 2 +- test/options/optimization-level-test.js | 8 +++---- test/source-map-test.js | 2 +- 13 files changed, 50 insertions(+), 25 deletions(-) diff --git a/History.md b/History.md index 042fc68d..c323c237 100644 --- a/History.md +++ b/History.md @@ -2,6 +2,7 @@ ================== * Adds more detailed error & warning messages on top of the new tokenizer. +* Disables restructuring optimizations by default until optimized in #533. * Fixes a bug ignoring incorrect properties in complex restructuring. * Requires Node.js 4.0+ to run. * Removes `benchmark` API option as total time is always reported under `stats` property. diff --git a/README.md b/README.md index 97d3aff4..70b898d2 100644 --- a/README.md +++ b/README.md @@ -130,7 +130,7 @@ Level 2 optimizations: cleancss -O2 one.css cleancss -O2 mediaMerging:off;restructuring:off;semanticMerging:on;shorthandCompacting:off one.css # `mediaMerging` controls `@media` merging behavior; defaults to `on` (alias to `true`) -# `restructuring` controls content restructuring behavior; defaults `on` (alias to `true`) +# `restructuring` controls content restructuring behavior; defaults `off` (alias to `false`) # `semanticMerging` controls semantic merging behavior; defaults to `off` (alias to `false`) # `shorthandCompacting` controls shorthand compacting behavior; defaults to `on` (alias to `true`) ``` @@ -195,7 +195,7 @@ new CleanCSS({ level: { 2: { mediaMerging: true, // controls `@media` merging behavior; defaults to true - restructuring: false, // controls content restructuring behavior; defaults to true + restructuring: false, // controls content restructuring behavior; defaults to false semanticMerging: false, // controls semantic merging behavior; defaults to false shorthandCompacting: true // controls shorthand compacting behavior; defaults to true } diff --git a/bin/cleancss b/bin/cleancss index c3070ee1..fd3c7ac4 100755 --- a/bin/cleancss +++ b/bin/cleancss @@ -46,7 +46,7 @@ commands.on('--help', function () { console.log(' %> cleancss -O2 one.css'); console.log(' %> cleancss -O2 mediaMerging:off;restructuring:off;semanticMerging:on;shorthandCompacting:off one.css'); console.log(' %> # `mediaMerging` controls `@media` merging behavior; defaults to `on` (alias to `true`)'); - console.log(' %> # `restructuring` controls content restructuring behavior; defaults to `on` (alias to `true`)'); + console.log(' %> # `restructuring` controls content restructuring behavior; defaults to `off` (alias to `false`)'); console.log(' %> # `semanticMerging` controls semantic merging behavior; defaults to `off` (alias to `false`)'); console.log(' %> # `shorthandCompacting` controls shorthand compacting behavior; defaults to `on` (alias to `true`)'); diff --git a/lib/options/optimization-level.js b/lib/options/optimization-level.js index db506a72..c708af9d 100644 --- a/lib/options/optimization-level.js +++ b/lib/options/optimization-level.js @@ -16,7 +16,7 @@ DEFAULTS[OptimizationLevel.One] = { }; DEFAULTS[OptimizationLevel.Two] = { mediaMerging: true, - restructuring: true, + restructuring: false, semanticMerging: false, shorthandCompacting: true }; diff --git a/test/batch-test.js b/test/batch-test.js index c3928c84..a2386a42 100644 --- a/test/batch-test.js +++ b/test/batch-test.js @@ -48,7 +48,11 @@ function batchContexts() { new CleanCSS({ compatibility: isIE7Mode ? 'ie7' : '*', keepBreaks: true, - level: 2 + level: { + 2: { + restructuring: true + } + } }).minify(data.input, this.callback.bind(null, data)); }, 'outputs right content': function (data, error, output) { @@ -60,7 +64,11 @@ function batchContexts() { new CleanCSS({ compatibility: isIE7Mode ? 'ie7' : '*', keepBreaks: true, - level: 2, + level: { + 2: { + restructuring: true + } + }, sourceMap: true }).minify(data.input, this.callback.bind(null, data)); }, @@ -71,7 +79,7 @@ function batchContexts() { 'minifying via CLI': { 'topic': function (data) { exec( - '__DIRECT__=1 ./bin/cleancss -b -O2 ' + (isIE7Mode ? '-c ie7 ' : '') + path.join(dir, filename), + '__DIRECT__=1 ./bin/cleancss -b -O2 restructuring:on ' + (isIE7Mode ? '-c ie7 ' : '') + path.join(dir, filename), { maxBuffer: 500 * 1024 }, this.callback.bind(null, data) ); diff --git a/test/binary-test.js b/test/binary-test.js index df62a4c2..6647f0a5 100644 --- a/test/binary-test.js +++ b/test/binary-test.js @@ -162,9 +162,9 @@ vows.describe('./bin/cleancss') }) }) .addBatch({ - 'skip restructuring optimizations': pipedContext('div{margin-top:0}.one{margin:0}.two{display:block;margin-top:0}', '-O2 restructuring:off', { + 'enable restructuring optimizations': pipedContext('div{margin-top:0}.one{margin:0}.two{display:block;margin-top:0}', '-O2 restructuring:on', { 'should do basic optimizations only': function (error, stdout) { - assert.equal(stdout, 'div{margin-top:0}.one{margin:0}.two{display:block;margin-top:0}'); + assert.equal(stdout, '.two,div{margin-top:0}.one{margin:0}.two{display:block}'); } }) }) diff --git a/test/integration-test.js b/test/integration-test.js index b8fa9c5a..de7ea438 100644 --- a/test/integration-test.js +++ b/test/integration-test.js @@ -2221,7 +2221,7 @@ vows.describe('integration tests') '.one{color:red;margin:0}.two{color:red}.one{margin:0}', '.one,.two{color:red}.one{margin:0}' ] - }, { level: 2 }) + }, { level: { 2: { restructuring: true } } }) ) .addBatch( optimizerContext('units - IE8 compatibility', { diff --git a/test/optimizer/merge-adjacent-test.js b/test/optimizer/merge-adjacent-test.js index b0c3fd58..8f3c6a2a 100644 --- a/test/optimizer/merge-adjacent-test.js +++ b/test/optimizer/merge-adjacent-test.js @@ -66,7 +66,7 @@ vows.describe('remove duplicates') ], 'two same bodies over a block': [ '.one{color:red}@media print{.two{display:block}}.three{color:red}', - '.one,.three{color:red}@media print{.two{display:block}}' + '.one{color:red}@media print{.two{display:block}}.three{color:red}' ], 'two rules with latter with suffix properties': [ 'a{display:none}a{display:none;visibility:hidden}', @@ -90,6 +90,14 @@ vows.describe('remove duplicates') ] }, { level: 2 }) ) + .addBatch( + optimizerContext('with level 2 on - and restructuring on', { + 'two same bodies over a block': [ + '.one{color:red}@media print{.two{display:block}}.three{color:red}', + '.one,.three{color:red}@media print{.two{display:block}}' + ], + }, { level: { 2: { restructuring: true } } }) + ) .addBatch( optimizerContext('with level 2 off', { 'same context': [ diff --git a/test/optimizer/merge-non-adjacent-by-body-test.js b/test/optimizer/merge-non-adjacent-by-body-test.js index e0c05bad..28099957 100644 --- a/test/optimizer/merge-non-adjacent-by-body-test.js +++ b/test/optimizer/merge-non-adjacent-by-body-test.js @@ -85,7 +85,7 @@ vows.describe('merge non djacent by body') '.block1__element{color:#000}.block1__element--modifier{color:red}.block2{color:#000;display:block;width:100%}' // '.block1__element,.block2{color:#000}.block1__element--modifier{color:red}.block2{display:block;width:100%}' - pending #588 ] - }, { level: { 2: { semanticMerging: true } } }) + }, { level: { 2: { restructuring: true, semanticMerging: true } } }) ) .addBatch( optimizerContext('IE8 compatibility', { diff --git a/test/optimizer/reduce-non-adjacent-test.js b/test/optimizer/reduce-non-adjacent-test.js index 345deb4d..4bfa3d81 100644 --- a/test/optimizer/reduce-non-adjacent-test.js +++ b/test/optimizer/reduce-non-adjacent-test.js @@ -10,7 +10,7 @@ vows.describe('remove duplicates') ], 'multiple selectors': [ 'a{padding:10px;margin:0;color:red}.one{color:red}a,p{color:red;padding:0}', - '.one,a,p{color:red}a{margin:0}a,p{padding:0}' + 'a{margin:0;color:red}.one{color:red}a,p{color:red;padding:0}' ], 'with one redefined property': [ 'a{color:red;display:block}.one{color:red}a{color:#fff;margin:2px}', @@ -36,11 +36,7 @@ vows.describe('remove duplicates') 'a{padding:10px}.one{color:red}a{}', 'a{padding:10px}.one{color:red}' ], - 'when overriden by a complex selector': [ - 'a{padding:10px;margin:0;color:red}.one{color:red}a,p{color:red;padding:0}', - '.one,a,p{color:red}a{margin:0}a,p{padding:0}' - ], - 'when overriden by complex selectors': [ + 'when overriden by a complex selectors': [ 'a{padding:10px;margin:0;color:red}.one{color:red}a,p{color:red;padding:0}.one,a{color:#fff}', 'a{margin:0}a,p{color:red;padding:0}.one,a{color:#fff}' ], @@ -110,13 +106,25 @@ vows.describe('remove duplicates') ] }, { level: 2 }) ) + .addBatch( + optimizerContext('level 2 on and restructuring on', { + 'multiple selectors': [ + 'a{padding:10px;margin:0;color:red}.one{color:red}a,p{color:red;padding:0}', + '.one,a,p{color:red}a{margin:0}a,p{padding:0}' + ], + 'when overriden by a complex selectors': [ + 'a{padding:10px;margin:0;color:red}.one{color:red}a,p{color:red;padding:0}.one,a{color:#fff}', + 'a{margin:0}a,p{color:red;padding:0}.one,a{color:#fff}' + ] + }, { level: { 2: { restructuring: true } } }) + ) .addBatch( optimizerContext('level 2 on and aggressive merging off', { 'non-adjacent with multi selectors': [ 'a{padding:10px;margin:0;color:red}.one{color:red}a,p{color:red;padding:0}', '.one,a,p{color:red}a{padding:10px;margin:0}a,p{padding:0}' ] - }, { aggressiveMerging: false, level: 2 }) + }, { aggressiveMerging: false, level: { 2: { restructuring: true } } }) ) .addBatch( optimizerContext('level 2 off', { diff --git a/test/optimizer/restructure-test.js b/test/optimizer/restructure-test.js index 012d55c0..95300c59 100644 --- a/test/optimizer/restructure-test.js +++ b/test/optimizer/restructure-test.js @@ -164,7 +164,7 @@ vows.describe('restructure') '.one{border-color:#000;border-bottom-color:rgb(0,0,0,.2)}.two{border-color:#fff;border-bottom-color:rgb(0,0,0,.2)}', '.one{border-color:#000;border-bottom-color:rgb(0,0,0,.2)}.two{border-color:#fff;border-bottom-color:rgb(0,0,0,.2)}' ] - }, { level: 2 }) + }, { level: { 2: { restructuring: true } } }) ) .addBatch( optimizerContext('level 2 off', { diff --git a/test/options/optimization-level-test.js b/test/options/optimization-level-test.js index 7c6a747d..924ae131 100644 --- a/test/options/optimization-level-test.js +++ b/test/options/optimization-level-test.js @@ -71,7 +71,7 @@ vows.describe(optimizationLevelFrom) 'has level 2 options': function (levelOptions) { assert.deepEqual(levelOptions['2'], { mediaMerging: true, - restructuring: true, + restructuring: false, semanticMerging: false, shorthandCompacting: true }); @@ -107,7 +107,7 @@ vows.describe(optimizationLevelFrom) 'has level 2 options': function (levelOptions) { assert.deepEqual(levelOptions['2'], { mediaMerging: true, - restructuring: true, + restructuring: false, semanticMerging: false, shorthandCompacting: true }); @@ -199,7 +199,7 @@ vows.describe(optimizationLevelFrom) 'has level 2 options': function (levelOptions) { assert.deepEqual(levelOptions['2'], { mediaMerging: false, - restructuring: true, + restructuring: false, semanticMerging: true, shorthandCompacting: true }); @@ -224,7 +224,7 @@ vows.describe(optimizationLevelFrom) 'has level 2 options': function (levelOptions) { assert.deepEqual(levelOptions['2'], { mediaMerging: false, - restructuring: true, + restructuring: false, semanticMerging: true, shorthandCompacting: true }); diff --git a/test/source-map-test.js b/test/source-map-test.js index 81f54b8a..0ae3c806 100644 --- a/test/source-map-test.js +++ b/test/source-map-test.js @@ -1816,7 +1816,7 @@ vows.describe('source-map') 'level 2 optimizations': { 'new property in restructuring': { 'topic': function () { - return new CleanCSS({ level: 2, sourceMap: true }).minify('a{color:#000}div{color:red}.one{display:block}.two{display:inline;color:red}'); + return new CleanCSS({ level: { 2: { restructuring: true } }, sourceMap: true }).minify('a{color:#000}div{color:red}.one{display:block}.two{display:inline;color:red}'); }, 'has right output': function (minified) { assert.equal(minified.styles, 'a{color:#000}.two,div{color:red}.one{display:block}.two{display:inline}'); -- 2.34.1