new API for plugin errors/warnings

[MoOx]

Currently postcss can encounter unrecoverable error during parsing, that might throw exceptions.
Next level should be to add a new feature to allow plugins to report error/warnings in a stack.
A plugin might return an error during the process() if the current css is not fitting the requirements (like custom-prop need a --stuff defined before using var(--stuff).
We can also imagine warnings if a plugin want to warn the user about something it's recommanded to do (see stylelint).

We can even imagine that plugins will be able to check postcss version & with the same api solve #163.

var p = require("postcss")
var result = p().use(/*...*/).process()
result.css // already existing
result.errors // [ ... ]
result.warnings // [ ... ]

The format should contain at least source (filename, l, c) & message + plugin ref.

We might also reuse node.error() and a new node.warning().

That would helps a lot for stylelint and MoOx/postcss-cssnext#64 as it might help a lot to make webpack loader more awesome (with real reportings, not just stupid console.log)

[ai]

Awesome idea! Result#warnings is must have.

But what should be in result.css on errors? I think errors must stop compilation and any other task.

Or what error handling process you suggest for PostCSS users like gulp-postcss?

[MoOx]

I see this as a non breaking enhancement to postcss. So css should still be what it can be. A partial output is better than no output at all.

• Unrecoverable error (from parsing or plugin) are very likely to throw exception. This kind of errors are not going to use errors/warnings stack.
• A plugin can notify an error to say "I won't do my job" or something like "You used an non recommanded syntax". This can be considered as a recoverable error.
• A plugin can notify a warning just to say "You can do better than that" or "Hey this is deprecated, please upgrade your code asap".

Using the errors and warnings stack should not break the process. We can see that as just a way to notify some messages with a given severity.
We can even create just a "stack" of message, and we can allow people to just provide a severity flag (PostCSS.ERROR, PostCSS.WARN, PostCSS.LOG).

A stack can be an array like this

[{
    filename: "/whatever.css"
    line: 1,
    column: 23,
    source: [Object] // related source if message send from a node
    severity: 0, // PostCSS.LOG,
    origin: "plugin-name",
    id: "sort of uniq & short id",
    message: "This is a message dude!"
},
{ /*...*/ } //...
]

Plugins like postcss-loader or gulp-postcss might be able to do what they want with that. We can imagine a option (callback) to postcss that would handle the stack when process is done, that would allow tools to show/save or do whatever they whan with those messages. By default we can provide a simple GNU like output

/filename:1:23: plugin-name ERROR: This is a message dude ("uniqid")

Other tools like a linter or wrapper might easily be able to provide enhanced output, including source of the error.

[ai]

What is different between “unimportant error” and “warning”? :)

As I understand there are 2 semantics: we do not compile CSS and show error (for example, in browser) and we compile CSS and show warning somewhere in console.

[MoOx]

• unrecoverable error: BOOM YOU BREAK EVERYTHING, css is not going to be ok (this is not part of the stack & should be an exception thrown)
• unimportant error: plugin won't do his job correctly, but other plugins can continue, css can be incomplete but valid
• warning: plugin is not happy and tells you (eg: depreciation notice, linter, etc)

But I understand that might me confusing. That's why I mention a simple message stack with severity in each message. We can also imagine a simple log helper (to debug?)

[ai]

But what behavior gulp-postcss or rails-postcss should do for unimportant error?

[ai]

Hm. You suggest debug helper rather that console.log to have some debug: true option in PostCSS#process?

[MoOx]

By default, maybe we should not do anything & let tool handle the result.messages array themselve.
Cli tool might want to do simple console.log, other tool will do what they want to suit their workflow (gulp: console.log, rails: no idea lol).

[ai]

Nobody will see console.log and they will have a question, why PostCSS plugin doesn’t work ;).

What is a benefits of silent errors for unknown variables or mixins?

[ai]

We ask gulp-postcss fail on result.error by default, but has special silent option to doo not fail (but I still doesn’t know user cases for it).

[MoOx]

What I want to offer here is a new feature that don't have any equivalent.

For now, we don't have anything and some plugin are already doing console.log that nobody see.

It's especially boring in a webpack workflow for example (because webpack dev server output a lot of thing, and "simple" console.log are mixed in a huge output - that said, it offers a nice way to emit messages that will be outputed at the end of build so user can see them).

I think it's a must have to provide a way so plugins can communicate with users in a normalized way. Using a simple result.messages array seems better than nothing.

With this new API, cli & wrapper might want to do whatever they want with the messages array (cli: console.log in an appropriate moment, wrapper: use corresponding api to show the messages)

[ai]

I totally agree. Question is only about result.messages. Maybe we should add only result.warnings and raise error instead of result.errors?

But maybe I miss some important user case.

[MoOx]

It's up to the plugin to decide if they want to throw or just notify. Plugins can already throw exceptions.
I think we should focus on a way to provide a mecanism so plugins can communicate with users.

We should not care plugin will use error, warnings, simple logs... We should just provide a generic way to returns message with a given type (severity can be PostCSSMessage.ERROR, PostCSSMessage.WARN, PostCSSMessage.LOG or other I don't see yet).
That's why I think result.messages seems more relevant than results.warnings or .errors.

[ai]

OK. I will add result.messages on next release.

[MoOx]

I can do something to help.
We need to decide how we can register messages.
I think we should offer a way to

• register a message not related to a node (directly from postcss instance, so maybe we should provide to plugin a 2nd parameter after the root node)
• register a message directly from a node via a simple node.message() (and do not change node.error() that throw)

We should also think about the message object. I check eslint and came with that

{
    filename: "/whatever.css"
    line: 1,
    column: 23,
    source: [Object] // related source if message send from a node
    severity: 0, // PostCSS.LOG,
    origin: "plugin-name",
    id: "sort of uniq & short id",
    message: "This is a message dude!"
}

What do you think about that?
Maybe plugins should be able to define the origin slug only once in the definition of the plugin ?
Something like plugin.name ?
The id should be a uniq key (in the scope of the plugin, so for postcss a id is "plugin origin+id").

[ai]

@MoOx yeeap, I think about postcss.plugin(opts, fn) definition for PostCSS version check.

[ai]

What is id key?

[MoOx]

A simple key to define a message. Message can be updated but the key might helps tools to adjust behavior according to those key (eg: ignore some messages).

[ai]

Maybe code? Because id cann be understand as unique message ID in messages.

[MoOx]

yep :)

[ai]

Very interesting things is where we will store the messages. We can use Root, but some plugins may replace AST root (for example, when we will concat two files one root will be missed).

Maybe we should add second argument to plugin functions? function (css, data) {}

Or forbid to change root?

[MoOx]

We should provide postcss instance as a second parameter to the plugin functions. That will be flexible enough to avoid changing plugin function signature later.

[ai]

What do you think about this API:

node.message(text, id);

Plugin name we will take from postcss.plugin wrap.

[ai]

Maybe we should rename mmethod to warning to prevent too big semantic meaning?

[ai]

Maybe we should have some id option in CssSyntaxError?

[MoOx]

Like I said many times, I think it's not just about warning. You should be able to define different type of severity.
We should also offer an API on processor instance directly so different plugins might specify a same origin (eg: stylelint will use many plugins but origin will be the same). Also we should be able to add data with the message.
So here are some idea that I would love to see

var severity = require("postcss/messages/severity")

processor.message("text message", {origin: "stylelint", severity: severity.INFO, source: node.source, customData: "...", })
// and with one arg only
processor.message({origin: "stylelint",  message: "text message", customData: "...", severity: severity.WARNING})
processor.message("text message"}) // severity: severity.INFO by default

// at node level
node.message(...) // same thing without the need for node and/or source

I think the origin from the plugin name is a good idea but it should be overwritable.
Also, I think we should support custom data, that will make things flexible. id will not make sense for common usage and might be considered as a custom data.

A message should have at least: text + origin. Other thing should be optional (source, severity, related node...)
Why do you think about that ?

[ai]

What other types of severity we will use?

Yeap, direct API is nececessary. Your processor.message API is nice.

And I am +1 for overwritable plugin name and flexible.

[MoOx]

INFO, WARNING, ERROR are enough for a start. People will still be able to provide custom severity value.
We can use something like keymirror for those types.

[ai]

I think errors should be raised by throw. Can you provide some example of INFO messages from plugins?

[MoOx]

There is 2 kind of errors:

• unrecoverable errors (mostly parsing error)
• recoverable errors (a plugin doesn't work, but other can continue the routine since it doesn't break the entire process)

[ai]

What is difference between “recoverable errors” and warnings?

[ai]

Maybe we can add low level message property.

But users will use Result#warning(text, data) to add message with type: warning and Result#warnings getter, that filter messages by type.

[MoOx]

A plugin that doesn't work vs. a plugin that warn you for a deprecation notice for example.

[ai]

Why plugin that doesn't work just can't send a warning? Why we need special type?

[ai]

I added Result#messages f892c6e

I will close this issue, when add Warning class with only toString() method.

[ai]

Warning class is finished 87f05c7

[davidtheclark]

@ai and @MoOx: Did you have some specific model in mind for how something like a linter, whose whole purpose is to show errors, would use warnings? I'm thinking about postcss-bem-linter, specifically, and have at least these two questions:

1. I can see that the warning can contain information that I could use to create a fancy message with line:column location, message, maybe even source. Will this kind of message tailoring (concatenating several properties of the warning) be done in different ways by each individual plugin or is there some standard way to do it?
2. I'd want the process to end with an exit status code of 2, I think --- but how is that supposed to work with runners? I could make the plugin provide its own status code, in case it were run alone; but then would runners respond correctly? Or do I need to have two modes to run in, one which just prints warnings and then continues with a 0 and one that exits with a 2 (for CI)?

(Ultimately I know these same questions will come up for other linters, including any written for stylelint as that gets started.)

[ai]

1. PostCSS will return just a Warning object. Each PostCSS runner can show warnings in their way (or example, with or without source code). But I think, most runners will use Warning#toString().
2. I prefer when linter throw a error. Maybe we should had a option: warning/throw a error?

[MoOx]

You should take a look to eslint.
A linter should not use an exit code nor throw an error. It should return an object with meta data that can be used by runners (cli, grunt/gulp/... plugins, webpack loaders etc) depending on what the user want.

[davidtheclark]

Ok, I get that. Unfortunately at this point it seems like adding another step in the chain (a tool that understands the metadata and responds accordingly) is an annoyance for a small independent focused linter like postcss-bem-linter. Maybe I should make the plugin itself work that way (outputting metadata) but then include a little CLI that interprets the metadata, prints things, throws errors, etc., enabling the plugin to be used on its own.