Fix complex spread props breaking (#329) #356
Conversation
There was a problem hiding this comment.
Awesome thank you @pudility! The branch looks good.
You can add the new test cases to the existing fixture test/fixtures/attribute-generation-classname-rewriting.js
Somewhere in that component, I would add the following:
<div className="test" {...test.test} />
<div className="test" {...test.test.test} />
<div className="test" {...this.test.test} />And then you can run npm test -- -u to update the snapshots. Remember to commit them obviously.
src/_utils.js
Outdated
| t.identifier(name), | ||
| t.identifier('className') | ||
| ) | ||
|
|
||
| if (node.argument.type === 'MemberExpression') { |
There was a problem hiding this comment.
here you can do the following:
let attrNameDotClassName
if (t.isMemberExpression(node.argument)) {
attrNameDotClassName = // with nested member expression
} else {
attrNameDotClassName = // with simple identifier
}
test/__snapshots__/index.js.snap
Outdated
| @@ -81,6 +81,47 @@ export default class { | |||
| }" | |||
| `; | |||
|
|
|||
| exports[`works with complex spread props (class) 1`] = ` | |||
There was a problem hiding this comment.
Can you revert the changes to this file? It seems that you removed the fixture and the test from test/index.js but the snapshot is still here in the diff :)
There was a problem hiding this comment.
I added back the tests in test/index.js, is that what you where looking for? If not I am not sure I understand what you are asking, sorry.
There was a problem hiding this comment.
Sorry I didn't remember correctly the tests for this feature are in test/attribute.js.
Can you do the following:
- remove the test from
test/index.js - regenerate the snapshot for
test/index.jslike so:
rm test/__snapshots__/index.js.snap
npm test -- -u
test/index.js
Outdated
| @@ -20,6 +20,13 @@ test('works with stateless', async t => { | |||
| t.snapshot(code) | |||
| }) | |||
|
|
|||
| test('works with spread', async t => { | |||
There was a problem hiding this comment.
we don't need this test here since it is already in test/attribute.js
test/__snapshots__/index.js.snap
Outdated
| @@ -81,6 +81,47 @@ export default class { | |||
| }" | |||
| `; | |||
|
|
|||
| exports[`works with complex spread props (class) 1`] = ` | |||
There was a problem hiding this comment.
Sorry I didn't remember correctly the tests for this feature are in test/attribute.js.
Can you do the following:
- remove the test from
test/index.js - regenerate the snapshot for
test/index.jslike so:
rm test/__snapshots__/index.js.snap
npm test -- -u
src/_utils.js
Outdated
| t.identifier(name), | ||
| t.identifier('className') | ||
| ) | ||
| } |
There was a problem hiding this comment.
nitpick: I just realized that this could be:
const attrNameDotClassName = t.memberExpression(
t.isMemberExpression(node.argument) ? node.argument : t.identifier(name),
t.identifier('className')
)
|
I think I got it all, but if there is anything else I can change, just let me know :) |
|
@pudility awesome, thank you again for contributing! |
|
@giuseppeg No problem, thanks for all the mentoring. |
Fixing issue #329.
TODO:
this.props.fooMemberExpressions