API walk* proxies don't work, and here’s how we fix them

[jonathantneal]

Hey, I’ve been a long-time user and advocate of postcss-values-parser. I’d like to think I’ve helped push it as the defacto value parser in PostCSS.

---

I’ve also been avoiding a significant bug in the API, and I’m here to resolve it with you. The walk proxies don’t work. Here’s why:

Container.registerWalker creates new proxy methods that walk nodes by a specific type. This means that something like Container.registerWalker(FunctionNode) becomes walkType(type, callback), where type is FunctionNode. So far, so good.

But within the walkType function, we normalize the type:

type = type.name && type.prototype ? type.name : type;

So now our type would be the following string; FunctionNode. And do you remember how we verify the node type?

if (node.type === type) { // node.type will be "func" and type will be "FunctionNode"

How do we fix this?

The fastest solution would be to update the walkType function like so:

walkType (type, callback) {
  if (!type || !callback) {
    throw new Error('Parameters {type} and {callback} are required.');
  }

  const isTypeCallable = typeof type === 'function'

  return this.walk((node, index) => {
    if (isTypeCallable && node instanceof type || !isTypeCallable && node.type === type) {
      return callback.call(this, node, index);
    }
  });
}

Would this be an acceptable solution? If so, I would gladly write up the PR.

I also want to acknowledge that @corysimmons and @nuintun tried to tell us in #34, #50, and #52. I just avoided the bug for a long time.

---

• Node Version: v10.0.0
• NPM Version: 6.0.0
• postcss-values-parser Version: 1.5.0

This issue is regarding a problem with:

• Standard CSS
• LESS
• SCSS
• SASS

Expected Behavior

The walk proxies walk nodes by their respective type

Actual Behavior

The walk proxies do not walk nodes by their respective type

How can we reproduce the behavior?

By testing the walk proxies

[shellscape]

Fantastic write up, thanks for contributing and for your advocacy 🎉. I'm totally open to.this PR. I've been admittedly short on time with a new marriage and a littany if additional open source work on webpack.

[corysimmons]

@shellscape Congrats on the marriage! :D