lodash / babel-plugin-lodash

Modular Lodash builds without the hassle.
Other
1.95k stars 95 forks source link

Picking up new react-bootstrap es/ modules build #114

Closed insin closed 8 years ago

insin commented 8 years ago

React Bootstrap just released 0.30.2, which includes an es/ build with native modules intact.

I have a failing test for cherry-picking with this plugin which creates an ES5 build and a native modules build, which is currently asserting that imports are cherry-picked from react-bootstrap/lib/ for both.

Is there a way for this plugin to know which module system transforms are in play (now that you can configure the es2015 preset) and choose the appropriate place to cherry-pick from based on conventions like lib/ being an ES5 build, or could it accept plugin config we could use to tell it where to cherry-pick from in the current build?

jdalton commented 8 years ago

Is there a way for this plugin to know which module system transforms are in play

I'm not sure. l know I'm not doing any such magic at the moment. I just updated my expected result it should still transpile fine.

Beyond the expected output path being different does the bundle fail in anyway?

taion commented 8 years ago

@jdalton That's not the correct result. require('react-bootstrap/es/Alert') doesn't make sense because react-bootstrap/es contains an ES module build with an actual export default: https://npmcdn.com/react-bootstrap@0.30.2/es/Alert.js

jdalton commented 8 years ago

Welp, then the plugin most likely no longer supports it at this time. Lodash's module builds are essentially the same thing regardless of export (amd, es, commonjs, node). The module format is just the window dressing around the code. I'll likely make paths more configurable. Right now its all auto-discovery (first match wins).

taion commented 8 years ago

I might be misunderstanding... to clarify, React-Bootstrap (like Redux and React Router) distributes to npm a poly-package. react-bootstrap/lib contains the CJS module build, while react-bootstrap/es contains the ES module build (and similarly for the other libraries). Right now, main points at lib/index.js while module and jsnext:main point at es/index.js.

In general we do things this way because it's a bit lighter on the tooling... these libraries all make CJS and UMD builds already anyway, so ES modules is just another build target that we ship in mostly the same way.

I think what @insin is asking is for some intelligence in resolving the import target – like resolving to lib/Foo when transpiling ES modules to CJS modules, but es/Foo otherwise.

Err... how are you picking up es/Alert anyway? We have main set to lib/index.js.

taion commented 8 years ago

Oh, looks like it's because es comes before lib lexicographically?

jdalton commented 8 years ago

I could dig a "root" option to specify where to search from. Would you be up for making a PR?

taion commented 8 years ago

I feel like the right way to go would be to look at the main entry point specified in package.json and start from the directory containing that file, instead of from the package root directory, and then optionally allow configuring the list of entry points to check.

Do you think that could work?

jdalton commented 8 years ago

Sure!

taion commented 8 years ago

@insin Since you opened this issue, do you want to take a stab at this? 😇 I'm merely an innocent bystander here.

insin commented 8 years ago

Yep, I'll have a look.

Defaulting based on main and having it configurable sounds good for when you explicitly want the ES6 modules version.

I need the latter when creating an ES6 modules build for React components and will probably want to switch from ES5 to ES6 modules for React apps once I upgrade to Webpack 2.

jdalton commented 8 years ago

Patched https://github.com/lodash/babel-plugin-lodash/commit/3706667bf163165c9c9d0266ca55f604eb05a2db.

taion commented 8 years ago

This is great, thanks – will get users in a state where things will work even if they're not using an ES module aware bundler.

danny-andrews commented 7 years ago

I could dig a "root" option to specify where to search from. Would you be up for making a PR?

Has this been added yet? I don't see it in 3706667. I'd be happy to take a stab at it if not.

For reference, my use-case is that I want to use the es6 build of recharts because it allows me to take advantage of tree-shaking in webpack.