Vague idea: add support for stack

[wooorm]

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Problem

• There’s a stack field on errors, which we we use when an error is passed (not super common I think)
• Those stacks on errors are about JavaScript code in transforms, and where things went wrong in that. This project is about errors inside some other processed file.
• While ASTs are not required to use vfile, they often are used together.

Solution

We could support stacks of nodes. E.g., hast/xast:

Error: Oh no!
    at text (example.html:1:1)
    at element<p> (example.html:1:1)
    at element<div> (example.html:1:1)
    at root (example.html:1:1)

mdast:

Error: Oh no!
    at text (example.html:1:1)
    at paragraph (example.html:1:1)
    at containerDirective (example.html:1:1)
    at root (example.html:1:1)

Alternatives

?

[ChristianMurphy]

I'm open to the idea.
We've broached the idea before in syntax-tree/mdast-util-to-markdown#55

Previously the concern was how verbose this could get in logs.
As well as the consideration that some of it (potentially most of it) could be caught ahead of time with the TypeScript typings.

[wooorm]

These concerns seem to be more about programmer errors in JavaScript. Plugins being configured wrong, or so. I think those errors should be thrown sync, not in a transformer, they don’t relate to a file or tree.

Here I am thinking more about lint messages. Someone forgot an alt on an element. Then we can show (up to 5 deep by default maybe?) where that image is, exactly.

[ChristianMurphy]

Ahhh, thanks for the clarification on the intent and planned use-case.
When I think stack, I usually think stack trace through a JS call stack, but here you mean something closer to a JSON path though the AST?
Would that be encoded as a path

["children", 1, "children", 0]

or would it include the raw nodes?

[
  {
    type: 'root',
    ...
  },
  {
    type: 'element',
    ...,
  },
  ...
}

[wooorm]

parents from unist-util-visit-parents could be used. Earlier nodes are higher up (root), later nodes are lower down (leaf that’s warned for). Given that their parents, it’s guaranteed that each node is a child of its parent, so, we can show a stack! :)

[remcohaszing]

I like the idea, but since vfile-message extends Error, I would expect stack to be the same thing in both. Maybe call it nodeStack? treeStack? ASTack 😄?

In order to get this stack, the VFile message needs to either:

• Be aware of the Node’s ancestry. This means it can only be reported in combination with unist-util-visit-parents.
• Be aware of the AST. It then needs to lookup the node in the AST.

I suggest a separate utility functions instead:

// Convenient for a single node.
/**
 * Get the stack of a node.
 *
 * @param {Node} node
 *   The node whose position to lookup in the AST.
 * @param {Node} ancestor
 *   The ancestor node to traverse to find the given node in. Typically this is a root node.
 * @returns {Node[] | undefined}
 *   The node’s ancestry within the given ancestor.
 */
function getNodeStack(node: Node, ancestor: Node): Node[] | undefined;

// This way we only need to traverse the ancestor once for multiple nodes.
/**
 * Get the stacks of multiple nodes.
 *
 * @param {Node[]} node
 *   The nodes whose position to lookup in the AST.
 * @param {Node} ancestor
 *   The ancestor node to traverse to find the given nodes in. Typically this is a root node.
 * @returns {Map<Node>}
 *   A map that maps the node to its ancestry within the given ancestor.
 */
function getNodeStacks(node: Node, ancestor: Node): Map<Node>;

// For presentation
/**
 * Convert a node stack to a string representation.
 *
 * @param {Node[]} nodes
 *   The nodes to represent in the stack.
 * @param {(node: Node) => string} [printer]
 *   Represent a node as a string. By default it will print the Node’s type and position.
 * @returns {string}
 *   A string that looks like a JavaScript stack trace.
 */
function printNodeStack(nodes: Node[], printer?: (node: Node) => string): string;

This way VFileMessage doesn’t need any additional context and can be used as before. VFile reporters can leverage this function.

The downside is users need to manually pull in this library to use it. It getNodeStack also be represented as a VFileMessage method, only accepting the ancestor option.

[wooorm]

since vfile-message extends Error, I would expect stack to be the same thing in both

Why? What is a stack, other than showing a trace of where something happened in JS, and who called who?
Here we are talking about mostly non-JS files, typically content, almost always ASTs, why not show the parents in that field?

Also .stack is typically about when an error is thrown, and most lint messages are not thrown.
So the next question: when would a lint message for some markdown using _ instead of * ever need a JS stack trace? Would that ever be useful? I can’t really imagine wanting to see a JS stack trace for ESLint messages.
The idea for reusing stack is that currently stack is never useful, and it’s better to have some useful value there.
But if a JS stack is sometimes useful, then we could just as well use a different field.

That being said, different engines might overwrite it, or not show it, as they might not accept tools setting it.
(Maybe of interest https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/stack)

In order to get this stack, the VFile message needs to either:

Yeah, I’m thinking allowing a list of nodes, where currently a point/position/node is accepted.
It could be [parent, node] which is almost always available, such as in unist-util-visit.

One more benefit for all this is that some nodes (perhaps the final node), might be somehow missing positional info:

Error: Oh no!
    at text (example.html:unknown)
    at element<a> (example.html:unknown)
    at element<p> (example.html:3:1-4:1)
    at element<div> (example.html:1:1-5:1)
    at root (example.html:1:1-12:1)

E.g., say @wooorm in a @wooorm b was replaced with a link, we currently can’t really warn for it, and while this isn’t perfect, it is improved.

---

Adjacent to this issue: code frames (https://babeljs.io/docs/en/babel-code-frame). I would really like to see that. But it’s tough to get right (when a reporter runs, the files have likely already been overwritten, and the file already likely has a different value).
It’s a tough different problem, but wanted to quickly mention it, as perhaps that solves the same problem but better.