-
Notifications
You must be signed in to change notification settings - Fork 3
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
Comments
I'm with the same error:
|
Hey! Sorry I'm just now seeing this. I am getting this error locally as well. I'm trying to debug now. |
No problem =D |
@vjpr and @Tautorn I confirmed this is happening (for me) when I use functional components that don't have a 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. 👍 |
Actually, wait. I'm investigating further. |
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. |
hello :) any news on this ? thx |
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 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 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 |
The text was updated successfully, but these errors were encountered: