Adding support for custom prop values

[andrewmtam]

Awesome tool you've built out here! One additional use case I've had for this scanner is to see what kinds of values are passed for props across the codebase for a specific component.

While your program handles most of the straightforward cases, it does not handle the case for when more complex properties are passed in as props.

If you're open to changing this behavior, I have a couple ideas!

Approach 1 -- Adding getPropValue to the configuration file

This PR covers the approach of making getPropValue configurable, so that consumers can format the props however they want, instead of using the defaults (which is great when the prop value is a Literal, but not as convenient for any other data type).

Here's what my config file looks like to get this working:

const escodegen = require("escodegen-wallaby");

module.exports = {
  getPropValue: (node) => {
    if (node.type === "JSXExpressionContainer") {
      return escodegen.generate(node.expression);
    } else {
      return escodegen.generate(node);
    }
  },
};

This tool escodegen-wallaby is very cool in that it takes an AST node, and actually converts it back to the literal code for that node! Also, I had to use escodegen-wallaby instead of escodegen because escodegen does not support JSX type nodes, where as escodegen-wallaby does

Approach 2 -- Changing getPropValue in scan.js to use escodegen-wallaby

For this approach, things would be more automated. If we just changed the core method:

https://github.com/andrewmtam/react-scanner/blob/75eb048c6315da5131d3a6fbc3c4cee1bbbbb14c/src/scan.js#L30-L49

to use escodegen-wallaby altogether, everyone would get this behavior out of the box!

Approach 3 -- Add a new configuration parameter, showDetailedProps

This is kind of a combination of Approach 1 and 2. If we want to keep getPropValue the way it is, for preserving backwards compatibility, but still like the idea of reporting the prop's full value, we can use a flag that switches between these methods.

Examples of new prop values

Using escodegen-wallaby, here's some examples of what could now be reported for props:

        "props": {
          "onClick": "() => {\n    if (someFunction) {\n        thenCallSomeFunction();\n    }\n    test.push(blah);\n}"
        },
...
        "props": {
          "loading": "loading",
          "onClick": "thisIsAVariableName"
        },
...
        "props": {
          "activeSortKey": "this.state.localReactState",
          "activeSortType": "this.state.moreLocalReactState",
          "sortKey": "key",
          "onClick": "sortKey => this.setSort(sortKey, sortFunc)"
        },

Let me know what you think!

[moroshko]

Thanks @andrewmtam for this!

Let's go with approach 1 which you implemented.

Some things that I'd change:

• Let getPropValue to accept component and prop names as well so that users could do "include expressions only for prop X in component Y). For example: getPropValue({ value, componentName, propName }). My gut feel that componentName should be the same as here.
• Add a new "Customizing prop values treatment" section in the README, describe the signature of getPropValue so people could see what parameters are available, and provide a real-world example to demonstrate how it works and what the output is. Link to this section from the Config file table where you mention getPropValue for the first time.
• Add this same example as a test. This way our tests are a bit more realistic and we ensure that what we say in the README works as expected.

Thanks again for your contribution ❤️

[andrewmtam]

One additional param I added here was the defaultGetPropValue, so that consumers can use the default handling where they don't care to specify a specific propName or componentName

[andrewmtam]

I added this as a dev dependency >.<, but its just for running the test. It just seems like this library is the easiest way to convert an ASTNode back into the actual code, without doing specific AST node type checks like if (node.type === 'ObjectExpression')...

[andrewmtam]

This seems like it should be equivalent to the fullCompnentName you linked in the comment. The .components. part is needed just because of the shape of the data structure, so if we strip out the .components. part and just join the componentParts with a ., this should reconstruct the actual value in JSX

[andrewmtam]

Sweet, thanks for the feedback! Updated the PR to reflect the requested changes; let me know if you'd like me to change anything else

[moroshko]

@andrewmtam Thanks a lot for this contribution!
I released 1.0.0 which contains this change.

[andrewmtam]

Np, thanks for the guidance and helping merge this in so quickly!