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

Use lodash-webpack-plugin. #1699

Closed
wants to merge 1 commit into from
Closed

Use lodash-webpack-plugin. #1699

wants to merge 1 commit into from

Conversation

jdalton
Copy link
Contributor

@jdalton jdalton commented May 6, 2016

I just released lodash-webpack-plugin.

demo

This would ensure you continue to get smaller modular builds and would allow you to expand your Lodash module use without worrying about the overhead of things like iteratee shorthands.

@timdorr
Copy link
Member

timdorr commented May 6, 2016

Neat! Been looking forward to this since I saw you posted about it on Twitter.

This gets a 👍 from me!

@rgbkrk
Copy link

rgbkrk commented May 7, 2016

Oh. My.

@gaearon
Copy link
Contributor

gaearon commented May 7, 2016

This would ensure you continue to get smaller modular builds and would allow you to expand your Lodash module use without worrying about the overhead of things like iteratee shorthands.

This would introduce a discrepancy between the CommonJS and the UMD builds because CommonJS builds wouldn’t work this way. So adding something like mapValue would then be cheap for UMD but expensive for CommonJS because we don’t use Webpack in the CommonJS build process.

To solve this, we could always bundle to a single file with Webpack, even for CommonJS. I’m not opposed to this, but it’s a breaking change, as we currently semi-support redux/lib/* style of imports. Now that Rollup and Webpack 2 are gaining traction, maybe we can remove support for redux/lib/* in the next breaking release as tree shaking solves the same problem more elegantly.

@gaearon gaearon added this to the 4.0 milestone May 7, 2016
@timdorr
Copy link
Member

timdorr commented May 7, 2016

The problem is that people use redux/lib/*-style imports to access "private" APIs. We get this more on react-router, where there are more of them to be found. If there is anything like that in Redux, you're now removing access to that code.

@gaearon
Copy link
Contributor

gaearon commented May 8, 2016

The problem is that people use redux/lib/*-style imports to access "private" APIs

They’re not quite private. I remember promising support for this use case some time during 2.x when tools like Rollup or Webpack 2 weren’t yet available, and some libraries that depended on Redux wanted smaller builds. So any top-level file under /lib/ is currently considered public API. (/lib/utils/* is not.)

@jdalton
Copy link
Contributor Author

jdalton commented May 8, 2016

@gaearon

So adding something like mapValue would then be cheap for UMD but expensive for CommonJS because we don’t use Webpack in the CommonJS build process.

I assume not all CommonJS use is to bundle. Couldn't you defer to the consumer to decide how they wish to bundle things. I mean, they may use webpack or browserify or rollup or uglify or closure-compiler or various other plugins and transforms. If they find size is an issue, they too could use lodash-webpack-plugin. With that I don't see this PR as needing a 4.0 bump.

@jdalton
Copy link
Contributor Author

jdalton commented Aug 10, 2016

I'm house cleaning lingering PRs.
Since it's trivial to add on your own and this PR has gone cold, it's...

@jdalton jdalton closed this Aug 10, 2016
@jdalton jdalton deleted the lodash-webpack-plugin branch August 10, 2016 19:25
@gaearon
Copy link
Contributor

gaearon commented Aug 11, 2016

Yea sorry, I’ve been busy with other projects and haven’t had a chance to take another look at this.
I still think that we can go with this only if:

To solve this, we could always bundle to a single file with Webpack, even for CommonJS.

@jdalton
Copy link
Contributor Author

jdalton commented Aug 11, 2016

No worries. I'm sure you'll get there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants