Add babel-plugin-lodash if app depends on Lodash

[gaearon]

Update: issue is taken by @iam4x. Please don't try to "finish it first"!

As seen in Automattic/wp-api-console#45, it seems like a good idea to do.

While we generally don't harcode support for libraries, Lodash is extremely popular to the point it can deserve special treatment, especially given that the plugin is easy to include conditionally and has significant benefits.

Our Babel preset could accept isUsingLodash as an option:

{
  "presets": ["react-app", {
    "isUsingLodash": true
  }]
}

It should be false by default.

Inside the preset, if isUsingLodash is true and the environment is production (we already have an if branch for environments there), we should require() and include babel-plugin-lodash (but not in other cases).

In Webpack config, we should conditionally pass isUsingLodash depending on whether app's package.json's dependencies contains Lodash of compatible versions.

Finally, after ejecting people who didn't depend on Lodash should have "presets": ["react-app"] in .babelrc, but people who used Lodash at the time they ejected should see "presets": [["react-app", { isUsingLodash": true }]] in it.

Update: issue is taken by @iam4x. Please don't try to "finish it first"!

[iam4x]

My turn to contribute, I will have time Tuesday 👍

[gaearon]

Sounds great, you got it!

[tmc]

This seems like an odd departure from the intentional simplicity of this project.

[gaearon]

Simplicity is in how to use it, not necessarily in how it works internally.
I would not even call how it works internally "simple" today.
What specific concerns do you have?

[gaearon]

To be clear this change has zero effect on the "public" API pre-ejecting or even post-ejecting if you don't use Lodash. If you do use it, you'll see one line in .babelrc being a few characters longer after ejecting. But it can make a big difference in bundle size and it's a great example of how we can make good choices for our users without them having to worry about it.

I just described how the feature should be implemented, not how it is used.

[alanhussey]

I haven't checked recently, but at one point babel-plugin-lodash did not support lodash's chaining syntax (_(data).filter(...).take(5) – is that a concern here? There could be CRA apps using that syntax which would be broken by upgrading to this.

[arunoda]

I tend to agree with @tmc
I think this is not directly related to this project. I got @gaearon's concern about bundle size and this is something doesn't affect the public API.

But there's already a fix available for this original issue. Just use the following syntax.

import pick from 'lodash/pick';

That's what recommends by lodash.

I think there could be more optimizations we could do like this. (may be in the future for different libraries). So, are we going to add those babel plugins as well?

---

Anyway, I think it's time to re-think about allowing the user to customize babel and webpack config.

[fdaciuk]

Just complementing @arunoda's post:
You may even use:

import pick from 'lodash.pick'

And just install lodash.pick instead all lodash.

So, it will be great if we had a way to customize webpack witout eject it :)

[havenchyk]

@fdaciuk from my perspective it will be a hell to manage a lot of lodash functions with such approach. The method from @arunoda is much better because it allows you just to have a single dependency, bundle size in this case will be small and package.json will be clean.

[gaearon]

But there's already a fix available for this original issue. Just use the following syntax.

I know that but many beginners don't.

I think there could be more optimizations we could do like this. (may be in the future for different libraries). So, are we going to add those babel plugins as well?

I would add them for very popular libraries. Let's not forget Lodash is (one of?) the most depended package on npm.

I think it's time to re-think about allowing the user to customize babel and webpack config.

Customising Babel config leads to hard-to-trace issues like https://github.com/cssinjs/react-jss/issues/32#issuecomment-190819245. Until Babel fixes such issues I am opposed to it.

Customising Webpack config is even more dangerous because we plan to migrate to Webpack 2 soon. So there would be breakage if you rely on the configuration format. Not to say changes like #1059 would be hard to introduce without breaking everyone. So no, I don't think this is a good idea for the project either at this time.

but at one point babel-plugin-lodash did not support lodash's chaining syntax

If it breaks the code we can't use it of course. My assumption is it doesn't but we need to check the issue tracker. However if by "doesn't work" you mean that it bails out of optimizing this is fine.

[julien-f]

👍 but I don't like the isUsingLodash flag, IMHO, babel-plugin-lodash should behave correctly if lodash is not used.

[gaearon]

I would love to hear more arguments for why we shouldn't do it but not the "slippery slope" kind. I think nothing prevents us from being reasonably smart and I'm open to adding more useful plugins if they can be enabled when you use a specific library, don't need configuration, and just make your code run better.