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

babel-plugin-lodash #1088

Closed
wants to merge 3 commits into from
Closed

babel-plugin-lodash #1088

wants to merge 3 commits into from

Conversation

iam4x
Copy link

@iam4x iam4x commented Nov 22, 2016

Linked to #1069

Work is in progress, since the discussion is getting bigger and bigger in the issue I'll wait the go from maintainers to continue and finish 👍

  • isUsingLodash option into babel-preset-react-app
  • automatically set isUsingLodash if lodash is found in app package.json
  • correctly set isUsingLodash option after eject

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions.

// plugins.push.apply(plugins, [
// require.resolve('babel-plugin-transform-react-constant-elements')
// ]);
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to move the whole block to provide a function to module.exports in order to have access to options object

"presets": [
["react-app", { "isUsingLodash": false } ]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know if it's really needed, followed @gaearon specs:

It should be false by default.

@@ -149,7 +154,7 @@ module.exports = {
// @remove-on-eject-begin
query: {
babelrc: false,
presets: [require.resolve('babel-preset-react-app')],
presets: [[require.resolve('babel-preset-react-app'), {isUsingLodash: isUsingLodash}]],
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to write only { isUsingLodash } but I'm not sure of which node versions are supported by react-create-app

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@gaearon
Copy link
Contributor

gaearon commented Dec 5, 2016

Thanks for PR! I changed my mind about isUsingLodash. We could just add the plugin separately in Babel config: #1069 (comment).

Now that I think of it, we don't need a flag at all. We can just add a plugin separately.

@gaearon
Copy link
Contributor

gaearon commented Dec 5, 2016

Let's hold off with the effort until we see any way to circumvent #1069 (comment).
Sorry about this!

@CrisLi
Copy link

CrisLi commented Feb 20, 2017

Any updates on this feature?

@gaearon
Copy link
Contributor

gaearon commented Feb 24, 2017

I think we won't be going ahead with this because #1069 (comment) is kind of a fatal flaw in this approach. _.chain is still a part of Lodash API. Happy to revisit this if Lodash removes _.chain so plugin is always safe to apply.

@gaearon gaearon closed this Feb 24, 2017
@lock lock bot locked and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants