Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
vjpr opened this issue Feb 16, 2018 · 9 comments
Open
Labels
help wanted Extra attention is needed

Comments

@vjpr
Copy link

vjpr commented Feb 16, 2018

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
Copy link

Tautorn commented Apr 8, 2018

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
Copy link
Owner

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

@Tautorn
Copy link

Tautorn commented Apr 30, 2018

No problem =D

@alanbsmith
Copy link
Owner

@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
Copy link
Owner

Actually, wait. I'm investigating further.

@alanbsmith
Copy link
Owner

alanbsmith commented Apr 30, 2018

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
Copy link

nihaux commented Jun 19, 2018

hello :) any news on this ? thx

@alanbsmith
Copy link
Owner

@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.

@alanbsmith alanbsmith added the help wanted Extra attention is needed label Jun 19, 2018
@gilbsgilbs
Copy link

gilbsgilbs commented Jun 19, 2018

@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:

  • How to iterate over every component in the AST? It would be quite trivial for class components (you'd just find classes inheriting Component and PureComponent), but how would it work for SFCs? Or even worse, for SFCs components declared as anonymous functions or for simple JSX constants (although we might skip those)?
  • Even if we succeed in detecting components reliably, how do we find the root node of the component? Let's take the following extreme example:
    class MyComponent extends React.Component {
      render() {
        if (Math.random() <= .5) return (<div>Some content</div>);
        return (<div>Some other content</div>);
      }
    }
    Theoretically, the data- attribute should be added to both div, but this is far from obvious to do. And it's even harder if the JSX comes from another function:
    class MyComponent extends React.Component {
      _render() {
        return (<div>Good stuff</div>);
      }
      render() {
        return this._render();
      }
    }

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants