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

ES module identifier imports throwing #69

Closed
jdlm-stripe opened this issue Aug 30, 2021 · 0 comments · Fixed by #70
Closed

ES module identifier imports throwing #69

jdlm-stripe opened this issue Aug 30, 2021 · 0 comments · Fixed by #70

Comments

@jdlm-stripe
Copy link
Contributor

Given a file with:

import FOO from './bar';

FOO;

And a babel config of:

module.exports = {
  plugins: [
    [
      "transform-define",
      {
        FOO: "bar"
      }
    ]
  ]
};

The plugin is currently crashing with:

TypeError: test/babel-plugins/define-transform/src/index.js: Property local of ImportDefaultSpecifier expected node to be of a type ["Identifier"] but instead got "StringLiteral"
    at Object.validate (node_modules/@babel/types/lib/definitions/utils.js:130:11)
    at validateField (@babel/types/lib/validators/validate.js:24:9)
    at Object.validate (@babel/types/lib/validators/validate.js:17:3)
    at NodePath._replaceWith (@babel/traverse/lib/path/replacement.js:147:7)
    at NodePath.replaceWith (@babel/traverse/lib/path/replacement.js:129:8)
    at replaceAndEvaluateNode (babel-plugin-transform-define/lib/index.js:37:12)
    at processNode (babel-plugin-transform-define/lib/index.js:62:5)
    at PluginPass.Identifier (babel-plugin-transform-define/lib/index.js:82:9)
    at newFn (@babel/traverse/lib/visitors.js:171:21)
    at NodePath._call (@babel/traverse/lib/path/context.js:53:20) {
  code: 'BABEL_TRANSFORM_ERROR'
}

Given that original file, I believe it should be unchanged if there's an ES module import in scope of the same name.

For context this is how webpack renders the same file:

<snip>
/*!**********************!*\
  !*** ./src/index.js ***!
  \**********************/
/*! no exports provided */
/***/ (function(module, __webpack_exports__, __webpack_require__) {

"use strict";
__webpack_require__.r(__webpack_exports__);
/* harmony import */ var _bar__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! ./bar */ "./src/bar.js");


_bar__WEBPACK_IMPORTED_MODULE_0__["default"];


/***/ })
<snip>

That's a little hard to read. Essentially I don't think this plugin should transform identifiers if they're defined as imports from other modules. I ran into this bug in a real-world code base.

I'll be putting up a PR with a fix shortly but wanted to file this issue first to make sure you folks are amenable to my proposed fix.

jdlm-stripe added a commit to jdlm-stripe/babel-plugin-transform-define that referenced this issue Aug 30, 2021
jdlm-stripe added a commit to jdlm-stripe/babel-plugin-transform-define that referenced this issue Aug 30, 2021
ryan-roemer pushed a commit that referenced this issue Sep 10, 2021
This PR fixes #69 by returning early during identifier traversal if it matches an ES module default or named import. I'm open to feedback if there's anything you think I missed!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant