kentcdodds / babel-plugin-macros

🎣 Allows you to build simple compile-time libraries
https://npm.im/babel-plugin-macros
MIT License
2.62k stars 135 forks source link

"Duplicate declaration" error due to identifier traversal change #164

Closed ben-rogerson closed 3 years ago

ben-rogerson commented 3 years ago

Hey Kent 👋

I'm experiencing an import issue in twin.macro that's popped up in the latest version of babel-plugin-macros (v3.0.0).

I've tracked down the change that's caused the break and noticed it wasn't described in the change list:

- state.file.scope.path.traverse({
+ traverse(state.file.ast, {
    Identifier() {},
})

It breaks with the following error:

./pages/index.tsx
TypeError: /Users/.../pages/index.tsx: Duplicate declaration "_styled" (This is an error on an internal node. Probably an internal error.)

The error hints at something else due to twin having inserted another styled-components/macro import. (A macro adding another macro ha!)

Here's the code that inserts the import in twin.macro:

twinImportPath.insertBefore(
    t.importDeclaration(
      [t.importDefaultSpecifier(generateUid('cssPropImport', program))],
      t.stringLiteral(styledComponentsMacroImport)
    )
)

Reference issue: https://github.com/ben-rogerson/twin.macro/issues/241

conartist6 commented 3 years ago

Here's what I gathered up from looking at the changes you mentioned: The change was in this PR. Which was made to solve this issue. And the whole thing is commented as a hack to solve this problem.

And it wasn't just a noisy change that can be reverted because that change was the one that solved the other bug. So I guess we need to understand the root cause of #121 so that we can devise an alternate fix for it.

conartist6 commented 3 years ago

I suspect what we're looking at is a regression of https://github.com/kentcdodds/import-all.macro/issues/7. The code that fixed the issue was rather magical, and according to the comment its intent was to do a traversal that would recompute scope. A stateless traversal of the ast alone would not recompute scope, making it pointless.

conartist6 commented 3 years ago

So to distill it down, the problem that was being seen was that somehow this code:

state.file.scope.path.traverse({
  Identifier() {},
})

Was causing a failure. It was never adequately investigated originally what the code would have interacted with to cause problems, but I'm pretty sure it must be something in this file. In that file we have const program = state.file.path, so both bits of code are working with the same objects. Looking through the file for the usages of program these lines jump out:

https://github.com/styled-components/babel-plugin-styled-components/blob/e3829d28f2b460e097f1499f6091a52b667ef1b4/src/visitors/transpileCssProp.js#L118-L124

conartist6 commented 3 years ago

As usual with babel the documentation consists of tweets: https://twitter.com/sebmck/status/1132038207309172736

ben-rogerson commented 3 years ago

Thanks so much for your fast response and epic digging Conrad 👍

github-actions[bot] commented 3 years ago

:tada: This issue has been resolved in version 3.0.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket: