Skip to content

Add babel plugin lodash #232

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

Add babel plugin lodash #232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
89d0a94
Add babel-plugin-lodash.
lukeasrodgers Feb 17, 2016
5e17f87
Import only the lodash functions we need.
lukeasrodgers Feb 17, 2016
963ae47
Fix js linter complaints in webpack config file.
lukeasrodgers Feb 18, 2016
699f7ea
Fix lodash find import.
lukeasrodgers Feb 18, 2016
fb06d87
Move lodash usage to `libs/slim_lodash.js`.
lukeasrodgers Feb 19, 2016
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/app/bundles/comments/components/CommentBox/CommentForm/CommentForm.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import Nav from 'react-bootstrap/lib/Nav';
import NavItem from 'react-bootstrap/lib/NavItem';
import Alert from 'react-bootstrap/lib/Alert';
import ReactCSSTransitionGroup from 'react/lib/ReactCSSTransitionGroup';
import _ from 'lodash';
import _ from 'libs/slim_lodash';

import BaseComponent from 'libs/components/BaseComponent';

Expand Down
2 changes: 1 addition & 1 deletion client/app/bundles/comments/components/CommentBox/CommentList/CommentList.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, { PropTypes } from 'react';
import Immutable from 'immutable';
import Alert from 'react-bootstrap/lib/Alert';
import ReactCSSTransitionGroup from 'react/lib/ReactCSSTransitionGroup';
import _ from 'lodash';
import _ from 'libs/slim_lodash';

import Comment from './Comment/Comment';
import BaseComponent from 'libs/components/BaseComponent';
Expand Down
2 changes: 1 addition & 1 deletion client/app/bundles/comments/components/SimpleCommentScreen/SimpleCommentScreen.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import Immutable from 'immutable';
import request from 'axios';
import _ from 'lodash';
import _ from 'libs/slim_lodash';

import metaTagsManager from 'libs/metaTagsManager';
import CommentForm from '../CommentBox/CommentForm/CommentForm';
Expand Down
2 changes: 1 addition & 1 deletion client/app/libs/metaTagsManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import _ from 'lodash';
import _ from 'libs/slim_lodash';

export default {

Expand Down
3 changes: 3 additions & 0 deletions client/app/libs/slim_lodash.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import bindAll from 'lodash/bindAll';
import find from 'lodash/find';
export default { bindAll, find };
Copy link

Choose a reason for hiding this comment

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

You shouldn't need to do this
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. Does this syntax seem correct to you?

import { bindAll, find } from 'lodash'; export default { bindAll, find };

This seems correct to me, and works fine on the dev build, but breaks the production build on this repo: Uncaught ReferenceError: bindAll is not defined. I unfortunately don't have enough experience with these technologies to be able to determine why.
Copy link

Choose a reason for hiding this comment

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

Not sure, I can reproduce though. Looking into it

The issue is that babel-plugin-lodash does not recognize/support export specifiers; I don't plan to support it either, when using the babel plugin you should use lodash as normal and let the plugin handle the grunt work of importing the modules directly. If you used the methods directly rather than building the slim_lodash file, babel-plugin-lodash would work as expected
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See previous discussion on the PR. The idea is to avoid including all of lodash when only specific functionality is required. I appreciate you taking time to look into this, thanks.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for clarifying @megawac.

@justin808 @alexfedoseev I'm going to close this PR. I'm not the right person to try to implement this functionality, I don't have enough experience with webpack/babel/es6 modules, and I think my efforts are just causing more confusion and wasting reviewers' time. I suspect it will be easy for someone else with more experience.
cheers
1 change: 1 addition & 0 deletions client/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
},
"devDependencies": {
"babel-eslint": "^5.0.0-beta8",
"babel-plugin-lodash": "^2.0.1",
"babel-plugin-react-transform": "^2.0.0",
"body-parser": "^1.15.0",
"chai": "^3.5.0",
Expand Down
3 changes: 3 additions & 0 deletions client/webpack.client.rails.build.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ config.module.loaders.push(
test: /\.jsx?$/,
loader: 'babel-loader',
exclude: /node_modules/,
query: {
plugins: ['lodash'],
},
},
{
test: /\.css$/,
Expand Down