formidablelabs / babel-plugin-transform-define Goto Github PK

Hey folks, thanks for creating this library, I found it very useful.

I'm opening this issue because I found an edge case related to how babel-loader cache works.

babel-loader uses a prop called cachedIdentifier. This property determinate if it is necessary or not re-run babel-loader process.

https://github.com/babel/babel-loader/blob/b29516266912fbc50120155fee00efb1b0a19f59/src/index.js#L61

The point here is that it reads the content of the .babelrc. All is okay.

The edge case is related with this:

{
  "plugins": [
    ["transform-define", "./path/to/config/file.js"]
  ]
}

If you use babel-plugin-transform-define specifying a file path, and then you modify the content of the file, in the next babel-loader it will be determinate that a new run is not necessary, so it will be served the last result from the cache that it's different from your expected result!

Literally, I was thinking I was crazy until discovering this.

The fix could be easiest as determinate the new way to calculate cachedIdentifier. The problem is, because in my case I'm under Next.js, that's is not something easy to do because Next.js internal doesn't contemplate this modification.

Ideally, we should do that transparently adding a bit of code inside babel-plugin-transform-define to detect the edge case, but not sure if we can do that, need to investigate around babel-loader API inside a plugin.

My current workaround is remove the cache before init the process:

rm -rf node_modules/.cache/babel-loader && npm run dev

but obviously, this is a very bad solution and slow.

any idea? 🙂

We're not using practically any babel features in src/index.js. Let's just write it in modern node:

• Move src/index.js over to lib/index.js as Node.js (fix exports)
• Remove lib in gitignore.
• Remove babel build steps at root .babelrc
• Adjust package.json:engines as appropriate

There's a bug that we can't defined some "false value", like false, 0, ''(empty string), undefined, null
just like following code
It will be ignored in babel compiling.

{
  "plugins": [
    ["transform-define", {
      "EMPTY_STRING": "",
      "FALSE_BOOLEAN": false,
      "ZERO_NUMBER": 0,
      "NULL": null
    }]
  ]
}

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.

Here is a babel config:

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

Transform the code below:

console.log(TEST); // should be transformed
function fn() {
  let TEST = "test"; // should not be transformed
  console.log(TEST); // should not be transformed
}

Currently, will be transformed into:

console.log("replaced");
function fn() {
  let "replaced" = "test"; // an error will be thrown here
  console.log("replaced");
}

Similar to #69 , an error will be thrown.

project structure

- __test__
- config
    - env
        - development.js
        - production.js
- src
.babelrc
.eslintrc
.nvmrc
package.json
...

babelrc

{
	"only": [
		"*.js",
		"*.babel.js",
		"*.test.js"
	],
	"ignore": [
		"./node_modules",
		"./node_modules/**/node_modules"
	],
	"presets": ["react", "env"],
	"env": {
		"development": {
			"plugins": [
				["transform-define", "./config/env/development.js"],
				"transform-class-properties",
				"transform-function-bind",
				"transform-object-rest-spread"
			]
		},
		"production": {
			"plugins": [
				["transform-define", "./config/env/production.js"],
				"transform-class-properties",
				"transform-function-bind",
				"transform-object-rest-spread"
			]
		}
	}
}

config

const UUID = require('common/utils/uuid');

const uuid = UUID();

const config = {
	port: 8080,
	urls: {
		database: <redacted>,
		api: 'http://localhost:8080/api',
		facebook: {
			callback: <redacted>
		}
	},
	identifiers: {
		cookie: {
			secret: <redacted>
		},
		session: {
			identifier: `${uuid}.sid`,
			secret: uuid
		}
	},
	auth: {
		facebook: {
			id: <redacted>,
			secret: <redacted>
		}
	},
	keys: {
		mandrill: <redacted>,
	}
};

module.exports = {
	APP_CONFIG: config
};

error
The nodePath: config/env/production.js is not valid.

Module build failed: Error: /Users/AAA/XXX/YYY/src/client/index.js: SyntaxError: Unexpected token export

Not sure why it's complaining about the path -- the file exists, and the path is correct. It seems like it doesn't like the way I'm exporting my config object? I don't want to have to whitelist a handful of globals in eslint just to use this, though.

https://snyk.io/vuln/npm:lodash:20180130

babel-plugin-transform-define uses 4.17.4 strictly.
Could you update to 4.17.5?

I was looking at some code recently that uses process?.env?.NODE_ENV and was curious if this plugin supported transforming this type of code and if so what was the expected behavior. I looked through the implementation and tests and it looks like this is not currently supported in any way, so I'm wondering what you think about OptionalMemberExpression and ideally how it would work.

If the configuration specifies process.env.NODE_ENV that I would expect it to transform process?.env?.NODE_ENV and any other variations using OptionalMemberExpression. What are your thoughts?

Thank you!

I suppose this is a way to go https://github.com/zeit/next.js/tree/canary/examples/with-dotenv

Maybe we can use memoize to improve performance by caching

Hey! Thanks for the plugin. There seems to be some problem when using it with React Native Reanimated (originally reported by @Titozzz). After some debugging, it turns out that babel-plugin-transform-define also tries to replace the names of JS object keys which is not expected. Here's the minimal reproducible example:

Input:

const obj = {
  __DEV__: __DEV__
};

Options:

module.exports = {
  presets: ["@babel/preset-env"],
  plugins: [
    ["babel-plugin-transform-define", {"__DEV__": true}]
  ]
};

Error:

TypeError: (...) Property key of ObjectProperty expected node to be of a type ["Identifier","StringLiteral","NumericLiteral"] but instead got "BooleanLiteral"

Expected:

const obj = {
  __DEV__: true
};

Note that it's possible to use [__DEV__] as the key, in such case it should be replaced.

When we define a config file like that :

{
    "plugins": [["transform-define", "./config/config.js"]]
}

We get the following error with babel 7 :

.plugins[1][1] must be an object, false, or undefined

The nuance is that actual|expected.js files need to be appropriately treated as generated output, not sources. But we can still harmonize.

• Update all eslint configs.
• Actually use the existing test/.eslintrc
• Probably add .eslintignore and switch to eslint . as package.json:scripts:lint

There is a common behaviour for many webpack plugins to require user specify baseDir in config instead of implicitly resolving path relatively to process.cwd.

Would you mind adding baseDir option to plugin config?

I have solved the issue myself but I thought it can be better.

It is good that it throws an error about it but the cryptic error message like this is not helping

remote: Module build failed: TypeError: /tmp/build_43fb00b7ca544651b6eb4730c0d0ad94/components/BuildTeam.js: Cannot convert undefined or null to object
remote:     at PluginPass.UnaryExpression (/tmp/build_43fb00b7ca544651b6eb4730c0d0ad94/node_modules/babel-plugin-transform-define/lib/index.js:34:27)
remote:     at newFn (/tmp/build_43fb00b7ca544651b6eb4730c0d0ad94/node_modules/babel-traverse/lib/visitors.js:276:21)
remote:     at NodePath._call (/tmp/build_43fb00b7ca544651b6eb4730c0d0ad94/node_modules/babel-traverse/lib/path/context.js:76:18)
remote:     at NodePath.call (/tmp/build_43fb00b7ca544651b6eb4730c0d0ad94/node_modules/babel-traverse/lib/path/context.js:48:17)
remote:     at NodePath.visit (/tmp/build_43fb00b7ca544651b6eb4730c0d0ad94/node_modules/babel-traverse/lib/path/context.js:105:12)
remote:     at TraversalContext.visitQueue (/tmp/build_43fb00b7ca544651b6eb4730c0d0ad94/node_modules/babel-traverse/lib/context.js:150:16)
remote:     at TraversalContext.visitSingle (/tmp/build_43fb00b7ca544651b6eb4730c0d0ad94/node_modules/babel-traverse/lib/context.js:108:19)
remote:     at TraversalContext.visit (/tmp/build_43fb00b7ca544651b6eb4730c0d0ad94/node_modules/babel-traverse/lib/context.js:192:19)
remote:     at Function.traverse.node (/tmp/build_43fb00b7ca544651b6eb4730c0d0ad94/node_modules/babel-traverse/lib/index.js:114:17)
remote:     at NodePath.visit (/tmp/build_43fb00b7ca544651b6eb4730c0d0ad94/node_modules/babel-traverse/lib/path/context.js:115:19)

This throws when the page it is saying at fault does not even use any of the variables supposed to be defined in env, but simply uses Unary Expression like "typeof".

Perhaps make the error clearer if the env passed in .babelrc is returning {} or null?

Thanks