fix: fix so that we dont blow up when encountering an "export default… #4
Conversation
|
I tried to also add a test that would cover the Edit: So looks like we don't have the options setup to use that. So it appears the |
ammmze
force-pushed
the
fix-default-export-class
branch
from
ab8f081
to
a508496
Compare
| path.replaceWith( | ||
| t.VariableDeclaration('const', [ | ||
| t.VariableDeclarator( | ||
| componentNameIdentifier, | ||
| iife.expression | ||
| ), | ||
| ]) | ||
| ) | ||
| const declarators = [ | ||
| t.VariableDeclarator( | ||
| componentNameIdentifier, | ||
| iife.expression | ||
| ), | ||
| ] | ||
| if ( | ||
| path.parentPath.isExportDefaultDeclaration() | ||
| ) { | ||
| path.parentPath.insertBefore( | ||
| t.VariableDeclaration( | ||
| 'const', | ||
| declarators | ||
| ) | ||
| ) | ||
| path.replaceWith(path.node.id) | ||
| } else { | ||
| path.replaceWith( | ||
| t.VariableDeclaration( | ||
| 'const', | ||
| declarators | ||
| ) | ||
| ) | ||
| } |
There was a problem hiding this comment.
I'm not very familiar with everything here, but I know enough to pretty much understand what i'm doing here. But I based this off another project that was facing the same issue.
… class ..." also adds tests to cover various scenarios
ammmze
force-pushed
the
fix-default-export-class
branch
from
a508496
to
50890bb
Compare
| } | ||
| } | ||
|
|
||
| _defineProperty(_class, 'displayName', 'FancyName1') |
There was a problem hiding this comment.
hmm...i need to look at this more ... our IIFE doesn't return the _class
There was a problem hiding this comment.
I've just pushed some updates that I think address the class wrappers. I had noticed that when we wrapped classes, the class never got returned from our IIFE. I couldn't figure out how to have it return the _class variable it was defining, and it felt weird having it return the class name but everything else was using this _class variable, so I changed it so it no longer changes the type to a ClassExpression. However, in doing so, we get an infinite loop, which I broke by just checking if the parent type is a BlockExpression.
|
Thanks for the PR! I probably won't have time to do a detailed review anytime soon, so my plan is to just run the tests and also try it in a real project when the PR is ready, and if everything works then I will just merge it. I can also publish it as a public npm package. Regarding the Jest issue with imports when you use |
Sounds like a plan! To keep things easier to maintain, I can also (in a separate PR) see about getting some github actions stuff setup to automate semver and publishing to npm.
Thanks! I had looked at that a little bit, but ultimately I ended up not needing the test for it since we don't currently wire up the options that would use the bit of code I was attempting to test, so its basically un-used code that was copied over |
ammmze
force-pushed
the
fix-default-export-class
branch
from
af94965
to
93a85ab
Compare
…ompatibility ideally we would be able to have it go through preset-env to get it to transform the arrow functions if necessary, but we dont appear to get that for free the way we are doing this. perhaps it can be investigated in the future
|
Hi, sorry for the delay. Is this ready to merge? I ran the tests and the demo works, so it looks good to me. I also updated the readme, etc. Future PRs for CI/CD are welcome. Thank you |
👍🏻 Yep! I think this is good to go. |
|
Published on npm as version 0.2.0! Thanks again. https://www.npmjs.com/package/babel-plugin-pure-static-props |
… class ..."
also adds tests to cover various scenarios
This is regarding the discussion we had over here.
But the gist of the issue is we blow up when a file contains a default export of a class:
The error we would receive was:
PS. I just saw your mention in #3, I'll take a deeper look at those tests as well.