FormidableLabs / babel-plugin-transform-define

Compile time code replacement for babel similar to Webpack's DefinePlugin
MIT License
245 stars 31 forks source link

Error when replacing with booleans #85

Closed tomekzaw closed 1 year ago

tomekzaw commented 1 year ago

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.

ryan-roemer commented 1 year ago

This behavior description is accurate.

So, we tackled things with bindings in https://github.com/FormidableLabs/babel-plugin-transform-define/issues/81

I'm not totally sure if we should skip object keys or if there are legitimate use cases to replace those.... Thoughts @carloskelly13 @gksander ?

As an aside, if you switch to string keys, you can get an equivalent, non-error output:

const obj = {
  "__DEV__": __DEV__
};

outputs to:

var obj = {
  "__DEV__": true
};
Titozzz commented 1 year ago

Thanks for opening that issue @tomekzaw .

I can see different cases:

const obj = {
  __DEV__
};
const obj = {
  __DEV__: __DEV__
};
const obj = {
  "__DEV__": __DEV__
};
const obj = {
  ["__DEV__"]: __DEV__
};

Should output:

var obj = {
  "__DEV__": true
};

Meanwhile

const obj = {
  [__DEV__]: __DEV__
};

Should be the only one that also affect the key

carloskelly13 commented 1 year ago

Agreed, in my view [__DEV__]: __DEV__ should see the key being replaced as the [] indicate to me, it is calculated where the others are just strings or non replaceable keys.

Titozzz commented 1 year ago

Hello there, gentle nudge to the issue, since I feel we all agree on the problem, what are the steps moving forward, should we expect you guys to fix it at some time or will you want a pull request? No idea what this issue would take to be fixed, but happy to give a hand!

cc @ryan-roemer @carloskelly13

Titozzz commented 1 year ago

Went ahead and created the PR

Titozzz commented 1 year ago

gentle nudge @ryan-roemer @carloskelly13 @gksander

gksander commented 1 year ago

Hi @Titozzz - thanks for resurfacing this. I'm a bit swamped this week, but will try to check this out first thing next week!

gksander commented 1 year ago

Okay, this fix has been released as 2.1.3! Thanks, all, for reporting, putting a solution together, and nudging this across the finish line!