Basic "plugin" options #316

nitriques · 2016-09-28T23:58:32Z

When usign the api, it is sometime necessary to act on the ast object
between csso operations. Instead of having to deal with options and calls to
parse and compress, this change makes it easy to edit the ast object
after csso's parse (afterParse) and compress (afterCompress) operation.

This is especially good for task runner, which can accept those
parameters to be able to speed task execution.

The new for a plugin came in view when I was copy/pasting code from csso or grunt-csso into my grunt task.
This is the experiment I've made: https://gist.github.com/nitriques/9d3b05c530a54c3a885c6bf41b46dbae

It's a re-implementation of purifycss but using async streams and csso instead of sync reads and rework. As per my tests (I've tested it on 4 productions sites) it's 45 times faster. Purify css yields saving of ~5%, while my grunt task saves more that 45% using the same config. Those are big wins.

I can live with the current experiment I've made, but feel like it would live better as a csso plugin, hence the PR.

If you want, I can create the release for you. All tests (both old and new) are passing. I would also happy to send a PR to the grunt-csso project too.

When usign the api, it is sometime necessary to act on the ast object between csso operations. Instead of having to deal with options and calls to parse and compress, this change makes it easy to edit the ast object after csso's parse (afterParse) and compress (afterCompress) operation. This is especially good for task runner, which can accept those parameters to be able to speed task execution.

coveralls · 2016-09-29T00:31:36Z

Coverage increased (+0.008%) to 98.258% when pulling 3eebcc1 on DeuxHuitHuit:plugins into 62bf720 on css:master.

coveralls · 2016-09-29T00:31:37Z

Coverage increased (+0.008%) to 98.258% when pulling 3eebcc1 on DeuxHuitHuit:plugins into 62bf720 on css:master.

lahmatiy · 2016-09-29T12:41:12Z

Thank you for your contribution!
I thought about the plugins system. This may allow further optimizations between steps. But it also may cause to less stability or performance penalty. Since now we are guaranteed to know the state of AST at each step but plugins can change it unpredictably.
Your sugestion is pretty much simple. But solves just few use cases. I'm not sure we should implement it that way. But I have no strong position here and need some time to make a decision.

lahmatiy · 2016-09-29T12:42:12Z

lib/index.js

@@ -60,6 +60,18 @@ function buildCompressOptions(options) {
    return options;
 }

+function runPlugins(ast, options, plugin) {
+    var plugins = [];
+    if (typeof options[plugin] === 'object' && options[plugin].length) {


if (Array.isArray(options[plugin])) {

Oh. Yeah, old habits are tough to lose..

Fixed in 69256e2

lahmatiy · 2016-09-29T12:43:22Z

lib/index.js

+    // after parse plugins
+    if (options.afterParse) {
+        debugOutput('afterParse', options, Date.now(),
+            runPlugins(ast, options, 'afterParse')


Better to pass options.afterParse instead of property name.

Hum indeed we could. I'll rewrite it.

Fixed in 1174a7e

lahmatiy · 2016-09-29T12:44:14Z

lib/index.js

@@ -75,11 +87,25 @@ function minify(context, source, options) {
        })
    );

+    // after parse plugins
+    if (options.afterParse) {


I think beforeCompress is much better name

Fixed in 87f1413

nitriques · 2016-09-29T15:00:05Z

But it also may cause to less stability or performance penalty. Since now we are guaranteed to know the state of AST at each step but plugins can change it unpredictably.

Understood. Maybe we could add a warning in the docs ? Is there a way to validate the AST after each plugin call ?

But solves just few use cases.

Which other case do you have in mind ?

coveralls · 2016-09-30T01:10:31Z

Coverage increased (+0.008%) to 98.258% when pulling 1174a7e on DeuxHuitHuit:plugins into 62bf720 on css:master.

coveralls · 2016-09-30T01:10:31Z

Coverage increased (+0.008%) to 98.258% when pulling 1174a7e on DeuxHuitHuit:plugins into 62bf720 on css:master.

nitriques · 2016-09-30T01:14:47Z

But it also may cause to less stability or performance penalty.

I've thought about it and I can't see how to simple if clause can slow things down. Adding plugins into a system always has a tradeoff, but current usage would not notice it. But I am available to talk about other options, like doing this change directly in grunt-csso, where it should only stop using minify and issue manual parse and compress operation. Thanks for this amazing little piece of code btw. ❤️ the List object.

nitriques · 2016-09-30T01:16:55Z

And I see this feature primarily used by runners... anything more complex would directly call parse and compress.

lahmatiy · 2016-09-30T09:56:27Z

Maybe we could add a warning in the docs ?

I thought about it. But I checked source and looks like adding beforeCompress is not so dangerous (I thought some things are not performing).

Which other case do you have in mind ?

Well some things may needs to be done after clean or compress steps, e.g. normalization, renaming and so on.

I've thought about it and I can't see how to simple if clause can slow things down.

Not directly but via adding more checking before or during step performing.

❤️ the List object.

It was desinged to avoid using arrays and better performance (see details). Parser and related things including List are moving to separate project - csstree. When csstree stabilizes csso will migrate on it.

nitriques · 2016-10-03T01:55:36Z

This is great thanks! Any ETA about publishing a new version on npm ?
Awesome slides BTW, read it all ;)

lahmatiy · 2016-10-03T10:10:41Z

I'm not sure this changes enough for release. I'll see what we can add and probably publish new version this week.

nitriques · 2016-10-04T01:14:02Z

Ok. Let's wait until I've tested a complete integration then. Working on the grunt-csso PR anyhow. Thanks again.

CSSO now supports callbacks to be able to poke with the AST before and afer compression. This allows developers to add more optimization on the AST before the file is written while avoiding to parse the css from disk. See css/csso#316 for details

lahmatiy · 2016-10-05T09:13:36Z

Keep in mind AST format will change soon. When format will be stabilized in csstree and csso migrates on it. Current work on migration in csstree branch.

nitriques · 2016-10-05T23:44:22Z

Perfect, thanks.

Should I send a new PR on the csstree branch ?

lahmatiy · 2016-10-06T00:23:38Z

Nope, I've already merge master to this branch.

nitriques · 2016-10-06T00:33:44Z

👍

nitriques · 2016-10-09T23:08:24Z

Well, I would finally need a release in order to be able to ship a passing test suite on the grunt plugin. If you could do it when you have a chance, that would be appreciated. Thanks :)

lahmatiy · 2016-10-25T11:20:44Z

Sorry for the wait. Published as 2.3.0

nitriques · 2016-10-25T15:00:56Z

No problems. Thanks!

nitriques force-pushed the plugins branch from 539ba67 to 6ae61e4 Compare September 29, 2016 00:01

nitriques force-pushed the plugins branch from 6ae61e4 to 3eebcc1 Compare September 29, 2016 00:29

lahmatiy reviewed Sep 29, 2016

View reviewed changes

nitriques added 3 commits September 29, 2016 20:59

Use Array.isArray() instead of typeof

69256e2

Rename afterParse to beforeCompress

87f1413

Pass the plugin directly, not the property name

1174a7e

lahmatiy merged commit c382cf5 into css:master Sep 30, 2016

nitriques mentioned this pull request Oct 5, 2016

Plugins support + Async I/O t32k/grunt-csso#27

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic "plugin" options #316

Basic "plugin" options #316

nitriques commented Sep 28, 2016

coveralls commented Sep 29, 2016

coveralls commented Sep 29, 2016 •

edited

Loading

lahmatiy commented Sep 29, 2016

lahmatiy Sep 29, 2016

nitriques Sep 29, 2016

nitriques Sep 30, 2016

lahmatiy Sep 29, 2016

nitriques Sep 29, 2016

nitriques Sep 30, 2016

lahmatiy Sep 29, 2016

nitriques Sep 29, 2016

nitriques Sep 30, 2016 •

edited

Loading

nitriques commented Sep 29, 2016

coveralls commented Sep 30, 2016

coveralls commented Sep 30, 2016 •

edited

Loading

nitriques commented Sep 30, 2016

nitriques commented Sep 30, 2016

lahmatiy commented Sep 30, 2016

nitriques commented Oct 3, 2016

lahmatiy commented Oct 3, 2016

nitriques commented Oct 4, 2016

lahmatiy commented Oct 5, 2016 •

edited

Loading

nitriques commented Oct 5, 2016

lahmatiy commented Oct 6, 2016

nitriques commented Oct 6, 2016

nitriques commented Oct 9, 2016

lahmatiy commented Oct 25, 2016

nitriques commented Oct 25, 2016

Basic "plugin" options #316

Basic "plugin" options #316

Conversation

nitriques commented Sep 28, 2016

coveralls commented Sep 29, 2016

coveralls commented Sep 29, 2016 • edited Loading

lahmatiy commented Sep 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nitriques Sep 30, 2016 • edited Loading

Choose a reason for hiding this comment

nitriques commented Sep 29, 2016

coveralls commented Sep 30, 2016

coveralls commented Sep 30, 2016 • edited Loading

nitriques commented Sep 30, 2016

nitriques commented Sep 30, 2016

lahmatiy commented Sep 30, 2016

nitriques commented Oct 3, 2016

lahmatiy commented Oct 3, 2016

nitriques commented Oct 4, 2016

lahmatiy commented Oct 5, 2016 • edited Loading

nitriques commented Oct 5, 2016

lahmatiy commented Oct 6, 2016

nitriques commented Oct 6, 2016

nitriques commented Oct 9, 2016

lahmatiy commented Oct 25, 2016

nitriques commented Oct 25, 2016

coveralls commented Sep 29, 2016 •

edited

Loading

nitriques Sep 30, 2016 •

edited

Loading

coveralls commented Sep 30, 2016 •

edited

Loading

lahmatiy commented Oct 5, 2016 •

edited

Loading