alanbsmith / babel-plugin-react-add-property

a Babel plugin for adding data attrs to React components (specifically for styled-components)
MIT License
20 stars 3 forks source link

TypeError: foo.js: Property value expected type of string but got null #3

Open vjpr opened 6 years ago

vjpr commented 6 years ago
xxx/node_modules/.registry.npmjs.org/babel-types/6.26.0/node_modules/babel-types/lib/definitions/index.js:161
      throw new TypeError("Property " + key + " expected type of " + type + " but got " + getType(val));
            ^
TypeError: foo.js: Property value expected type of string but got null
    at Object.validate (xxx/node_modules/.registry.npmjs.org/babel-types/6.26.0/node_modules/babel-types/lib/definitions/index.js:161:13)
    at validate (xxx/node_modules/.registry.npmjs.org/babel-types/6.26.0/node_modules/babel-types/lib/index.js:505:9)
    at Object.builder (xxx/node_modules/.registry.npmjs.org/babel-types/6.26.0/node_modules/babel-types/lib/index.js:466:7)
    at JSXElement (xxx/node_modules/.registry.npmjs.org/babel-plugin-react-add-property/0.1.2/node_modules/babel-plugin-react-add-property/lib/index.js:24:99)
    at NodePath._call (xxx/node_modules/.registry.npmjs.org/babel-traverse/6.26.0/node_modules/babel-traverse/lib/path/context.js:76:18)
    at NodePath.call (xxx/node_modules/.registry.npmjs.org/babel-traverse/6.26.0/node_modules/babel-traverse/lib/path/context.js:48:17)
    at NodePath.visit (xxx/node_modules/.registry.npmjs.org/babel-traverse/6.26.0/node_modules/babel-traverse/lib/path/context.js:105:12)
    at TraversalContext.visitQueue (xxx/node_modules/.registry.npmjs.org/babel-traverse/6.26.0/node_modules/babel-traverse/lib/context.js:150:16)
    at TraversalContext.visitSingle (xxx/node_modules/.registry.npmjs.org/babel-traverse/6.26.0/node_modules/babel-traverse/lib/context.js:108:19)
    at TraversalContext.visit (xxx/node_modules/.registry.npmjs.org/babel-traverse/6.26.0/node_modules/babel-traverse/lib/context.js:192:19)
    at Function.traverse.node (xxx/node_modules/.registry.npmjs.org/babel-traverse/6.26.0/node_modules/babel-traverse/lib/index.js:114:17)
    at NodePath.visit (xxx/node_modules/.registry.npmjs.org/babel-traverse/6.26.0/node_modules/babel-traverse/lib/path/context.js:115:19)
    at TraversalContext.visitQueue (xxx/node_modules/.registry.npmjs.org/babel-traverse/6.26.0/node_modules/babel-traverse/lib/context.js:150:16)
    at TraversalContext.visitMultiple (xxx/node_modules/.registry.npmjs.org/babel-traverse/6.26.0/node_modules/babel-traverse/lib/context.js:103:17)
    at TraversalContext.visit (xxx/node_modules/.registry.npmjs.org/babel-traverse/6.26.0/node_modules/babel-traverse/lib/context.js:190:19)
    at Function.traverse.node (xxx/node_modules/.registry.npmjs.org/babel-traverse/6.26.0/node_modules/babel-traverse/lib/index.js:114:17)
    at NodePath.visit (xxx/node_modules/.registry.npmjs.org/babel-traverse/6.26.0/node_modules/babel-traverse/lib/path/context.js:115:19)
    at TraversalContext.visitQueue (xxx/node_modules/.registry.npmjs.org/babel-traverse/6.26.0/node_modules/babel-traverse/lib/context.js:150:16)
    at TraversalContext.visitSingle (xxx/node_modules/.registry.npmjs.org/babel-traverse/6.26.0/node_modules/babel-traverse/lib/context.js:108:19)
    at TraversalContext.visit (xxx/node_modules/.registry.npmjs.org/babel-traverse/6.26.0/node_modules/babel-traverse/lib/context.js:192:19)
    at Function.traverse.node (xxx/node_modules/.registry.npmjs.org/babel-traverse/6.26.0/node_modules/babel-traverse/lib/index.js:114:17)
    at NodePath.visit (xxx/node_modules/.registry.npmjs.org/babel-traverse/6.26.0/node_modules/babel-traverse/lib/path/context.js:115:19)
Tautorn commented 6 years ago

I'm with the same error:

ERROR in ./src/index.jsx
Module build failed: TypeError: /home/bruno/Projects/xxx/crm/src/index.jsx: Property value expected type of string but got null
    at Object.validate (/home/bruno/Projects/xxx/crm/node_modules/babel-types/lib/definitions/index.js:161:13)
    at validate (/home/bruno/Projects/xxx/crm/node_modules/babel-types/lib/index.js:505:9)
    at Object.builder (/home/bruno/Projects/xxx/crm/node_modules/babel-types/lib/index.js:466:7)
    at JSXElement (/home/bruno/Projects/xxx/crm/node_modules/babel-plugin-react-add-property/lib/index.js:24:99)
    at NodePath._call (/home/bruno/Projects/xxx/crm/node_modules/babel-traverse/lib/path/context.js:76:18)
    at NodePath.call (/home/bruno/Projects/xxx/crm/node_modules/babel-traverse/lib/path/context.js:48:17)
    at NodePath.visit (/home/bruno/Projects/xxx/crm/node_modules/babel-traverse/lib/path/context.js:105:12)
    at TraversalContext.visitQueue (/home/bruno/Projects/xxx/crm/node_modules/babel-traverse/lib/context.js:150:16)
    at TraversalContext.visitMultiple (/home/bruno/Projects/xxx/crm/node_modules/babel-traverse/lib/context.js:103:17)
    at TraversalContext.visit (/home/bruno/Projects/xxx/crm/node_modules/babel-traverse/lib/context.js:190:19)
    at Function.traverse.node (/home/bruno/Projects/xxx/crm/node_modules/babel-traverse/lib/index.js:114:17)
    at NodePath.visit (/home/bruno/Projects/xxx/crm/node_modules/babel-traverse/lib/path/context.js:115:19)
    at TraversalContext.visitQueue (/home/bruno/Projects/xxx/crm/node_modules/babel-traverse/lib/context.js:150:16)
    at TraversalContext.visitSingle (/home/bruno/Projects/xxx/crm/node_modules/babel-traverse/lib/context.js:108:19)
    at TraversalContext.visit (/home/bruno/Projects/xxx/crm/node_modules/babel-traverse/lib/context.js:192:19)
    at Function.traverse.node (/home/bruno/Projects/xxx/crm/node_modules/babel-traverse/lib/index.js:114:17)
 @ multi (webpack)-dev-server/client?http://localhost:8080 regenerator-runtime/runtime ./src/index.jsx

.babelrc:

{
  "presets": ["env", "react", "stage-0"],
  "env": {
    "development": {
      "plugins": [
        "transform-decorators-legacy",
        ["babel-plugin-react-add-property", { "property": "data-qa" }],
        ["babel-plugin-styled-components", {
          "fileName": false
        }]
      ]
    }
  }
}
alanbsmith commented 6 years ago

Hey! Sorry I'm just now seeing this. I am getting this error locally as well. I'm trying to debug now.

Tautorn commented 6 years ago

No problem =D

alanbsmith commented 6 years ago

@vjpr and @Tautorn I confirmed this is happening (for me) when I use functional components that don't have a displayName. This lib uses the displayNames to create the data attrs.

e.g.

function App() {
  return <div>this will break</div>;
}

I'll work on a patch, but until then, you should be able to do this to resolve the issue:

function App() {
  return <div>this will break</div>;
}

App.displayName = 'App';

Thanks for raising the issue! I'll let you know when I have a fix published. 👍

alanbsmith commented 6 years ago

Actually, wait. I'm investigating further.

alanbsmith commented 6 years ago

Okay, it looks like something has changes with how Babel is traversing these nodes, but I'm not sure why. I have a patch that's working locally. I'll push it tonight. Thank you for being patient.

EDIT: Looks like my understanding of the problem was faulty. I need to do more research to resolve this issue.

nihaux commented 6 years ago

hello :) any news on this ? thx

alanbsmith commented 6 years ago

@nihaux,

It appears to be a bit more complicated than I anticipated.

It looks like this plugin works fine for styled-components, but I haven't found a way to get it to work with other React component types. 😞

I'm fairly sure it's possible, but I haven't had time to research and explore. Happy to have help on this if someone has some time.

gilbsgilbs commented 6 years ago

@alanbsmith Has this plugin ever worked as expected in the past (outside of styled-components)? Unfortunately, there's no test written for this plugin, therefore I don't really know what is the real expected behavior. I might be wrong, but as I read it, it's supposed to add an attribute containing the node name to every single JSX node in the AST, but provided that only component's content is effectively rendered to the DOM, this wouldn't be really useful. It would just add data-test="div" on divs on so on. Am I missing something?

The only way around I see would be to iterate over every component and add the attribute only to the root node of each of them. Meaning that there are at least two hard problems to solve:

Again, I might be wrong and miss something obvious. Yet with the information I have, I'm not sure it's a problem we could solve with just 24 lines of JS code (and not sure it's a problem we could solve reliably at all, if working as specified). Although I recognize it would be quite useful for simplifying E2E testing.

I'm under the impression that it would be a lot easier (although a lot less elegant) to patch the render method or something like that.

Edit: For example, it would be quite feasible to patch class components this way:

// this

class MyComponent extends React.Component {
  render() {
    return <div><span>FooBar</span></div>;
  }
}

// would transpile to

class MyComponent extends Component {
  _7b1e8437_render() {
      return <div><span>FooBar</span></div>;
  }

  render() {
    const result = this._7b1e8437_render();
    return React.createElement(
      result.type,
      {
        ...result.props,
        'data-test': 'MyComponent',
      }
    );
  }
}

Doesn't solve the SFC detection though. And wouldn't work when the root component is a Fragment (don't know what would be the expectation in such case though). And it looks quite scary and risky (might break in some edge-cases I don't see and would anyways require very thoughtful and deep testing).