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

Error when replacing with booleans #85

Closed
tomekzaw opened this issue Jun 5, 2023 · 8 comments · Fixed by #86
Closed

Error when replacing with booleans #85

tomekzaw opened this issue Jun 5, 2023 · 8 comments · Fixed by #86

Comments

@tomekzaw
Copy link

tomekzaw commented Jun 5, 2023

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
Copy link
Member

This behavior description is accurate.

So, we tackled things with bindings in #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
Copy link
Contributor

Titozzz commented Jun 5, 2023

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
Copy link

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
Copy link
Contributor

Titozzz commented Jun 30, 2023

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
Copy link
Contributor

Titozzz commented Jul 4, 2023

Went ahead and created the PR

@Titozzz
Copy link
Contributor

Titozzz commented Jul 27, 2023

gentle nudge @ryan-roemer @carloskelly13 @gksander

@gksander
Copy link
Contributor

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
Copy link
Contributor

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!

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.

5 participants