Transformer should not rely on mutated object

[dthree]

Take the following abbreviated sample of an embedded plugin:

// This will only return the first element in the .md
const processor = mdast().use(function (mdst, opt) {
  function transformer(ast, file) {
    ast.children = ast.children.slice(0, 1);
  }
  return transformer;
});
return processor.process(data);

In this example, the transformer method is expected to mutate the incoming parameters, ast and file. This had me confused for quite a while as it is commonly considered a best-practice to keep parameters immutable. Due to expecting transformer to return the tranformed objects and not seeing it in any of your plugins, I was thrown a bit. The transformer doesn't actually do anything with its returned object.

A more optimum approach would be something like this:

// This will only return the first element in the .md
const processor = mdast().use(function (mdst, opt) {
  function transformer(ast, file) {
    var mutatedAst = ast.children.slice(0, 1);
    return mutatedAst;
  }
  return transformer;
});
return processor.process(data);

While I understand two parameters are in play, they should probably be returned grouped together as an object. The point is that one should not expect the user to mutate incoming parameters and not even return a result, which is a basic in functional programming.

---

I know correcting this would probably break other plugins: perhaps you could schedule it in to the next major release?

[wooorm]

Well, yes. That would break every plugin out there :) In addition, though I like the concept of immutability, this is how ware (which is used internally), koa, and express also use.

Also, it is impossible with the current set-up, through the previously mentioned ware, to add this, because it doesn’t support return values (P.S. transformers can be async)

In addition, often, plug-ins only make small changes. Adding a few attributes, &c. Wouldn’t it be very expensive in performance to have to clone the AST each time?

[dthree]

Okay fine I see where you're coming from :)

Maybe just a doc update that clearly explains that's what is happening would suffice. The main point is predictability, so if its clear that shouldn't be a problem IMHO.

[wooorm]

Tsk, a comma separated close doesn’t work 😦...

Anyway, I think that 8664a03 closes this because it clearly shows that no AST is returned. Do you agree? If not, please re-open and let me know where you’d like this notice!

[dthree]

Looks good! Thanks!