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

(wip) added eslint #280

Merged
merged 7 commits into from
May 4, 2016
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

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: 2 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
test/babel/fixtures
lib
16 changes: 16 additions & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"extends": "airbnb",
"parserOptions": {
"ecmaFeatures": {
"experimentalObjectRestSpread": true
},
},
"rules": {
"no-underscore-dangle": ["error", { "allow": ["__REACT_HOT_LOADER__"] }],
"no-console": ["error", { allow: ["error"] }],
"strict": 0,
},
globals: {
"__REACT_HOT_LOADER__": true
}
}
3 changes: 3 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,6 @@ language: node_js
node_js:
- "5"
- "6"
script:
- npm run lint
- npm test
2 changes: 1 addition & 1 deletion babel.js
Original file line number Diff line number Diff line change
@@ -1 +1 @@
module.exports = require('./lib/babel');
module.exports = require('./lib/babel');
2 changes: 1 addition & 1 deletion index.js
Original file line number Diff line number Diff line change
@@ -1 +1 @@
module.exports = require('./lib/index');
module.exports = require('./lib/index');
6 changes: 6 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"build": "babel src --out-dir lib",
"test": "mocha --compilers js:babel-core/register ./test",
"test:watch": "npm run test -- --watch",
"lint": "eslint .",
"prepublish": "npm run clean && npm run build"
},
"dependencies": {
Expand Down Expand Up @@ -52,6 +53,11 @@
"babel-preset-es2015": "^6.6.0",
"babel-preset-react": "^6.5.0",
"enzyme": "^2.2.0",
"eslint": "^2.9.0",
"eslint-config-airbnb": "^8.0.0",
"eslint-plugin-import": "^1.6.1",
"eslint-plugin-jsx-a11y": "^1.0.4",
"eslint-plugin-react": "^5.0.1",
"expect": "^1.16.0",
"jsdom": "^8.4.1",
"mocha": "^2.4.5",
Expand Down
2 changes: 1 addition & 1 deletion patch.js
Original file line number Diff line number Diff line change
@@ -1 +1 @@
module.exports = require('./lib/patch');
module.exports = require('./lib/patch');
40 changes: 24 additions & 16 deletions src/AppContainer.dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ class AppContainer extends Component {
}
}

componentWillReceiveProps(nextProps) {
componentWillReceiveProps() {
// Hot reload is happening.
// Retry rendering!
this.setState({
error: null
error: null,
});
// Force-update the whole tree, including
// components that refuse to update.
Expand All @@ -40,9 +40,9 @@ class AppContainer extends Component {
// In 15.0, it only catches errors on initial mount.
// Later it will work for updates as well:
// https://github.com/facebook/react/pull/6020
unstable_handleError(error) {
unstable_handleError(error) { // eslint-disable-line camelcase
this.setState({
error: error
error,
});
}

Expand All @@ -54,38 +54,46 @@ class AppContainer extends Component {

if (this.props.component) {
return <this.props.component {...this.props.props} />;
} else {
return React.Children.only(this.props.children);
}

return React.Children.only(this.props.children);
}
}

AppContainer.propTypes = {
component: function (props) {
component(props) {
if (props.component) {
return new Error(
`Passing "component" prop to <AppContainer /> is deprecated. ` +
`Replace <AppContainer component={App} /> with <AppContainer><App /></AppContainer>.`
'Passing "component" prop to <AppContainer /> is deprecated. ' +
'Replace <AppContainer component={App} /> with <AppContainer><App /></AppContainer>.'
);
}

return undefined;
},
props: function (props) {
props(props) {
if (props.props) {
return new Error(
`Passing "props" prop to <AppContainer /> is deprecated. ` +
`Replace <AppContainer component={App} props={{ myProp: myValue }} /> with <AppContainer><App myProp={myValue} /></AppContainer>.`
'Passing "props" prop to <AppContainer /> is deprecated. ' +
'Replace <AppContainer component={App} props={{ myProp: myValue }} /> ' +
'with <AppContainer><App myProp={myValue} /></AppContainer>.'
);
}

return undefined;
},
children: function (props) {
children(props) {
if (React.Children.count(props.children) !== 1) {
return new Error(`Invalid prop "children" supplied to AppContainer. Expected a single React element with your app’s root component, e.g. <App />.`);
return new Error('Invalid prop "children" supplied to AppContainer.' +
Copy link
Owner

Choose a reason for hiding this comment

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

There’s a missing space after the period in the error message. The sentences will collide.
Also a style nit: let’s put a newline right after the opening paren.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch

'Expected a single React element with your app’s root component, e.g. <App />.');
}
}

return undefined;
Copy link
Owner

Choose a reason for hiding this comment

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

Lol this rule is so nitpicky. But I can live with it 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah this one looks pretty silly in some places :)

},
};

AppContainer.defaultProps = {
errorReporter: Redbox
errorReporter: Redbox,
};

module.exports = AppContainer;
2 changes: 2 additions & 0 deletions src/AppContainer.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/* eslint-disable global-require */

'use strict';

if (process.env.NODE_ENV === 'production') {
Expand Down
2 changes: 2 additions & 0 deletions src/AppContainer.prod.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/* eslint-disable react/prop-types */

'use strict';

const React = require('react');
Expand Down
61 changes: 32 additions & 29 deletions src/babel/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const buildTagger = template(`
})();
`);

module.exports = function(args) {
module.exports = function plugin(args) {
// This is a Babel plugin, but the user put it in the Webpack config.
if (this && this.callback) {
throw new Error(
Expand All @@ -38,27 +38,28 @@ module.exports = function(args) {
// Try our best to avoid variables from require().
// Ideally we only want to find components defined by the user.
function shouldRegisterBinding(binding) {
let { type, node } = binding.path;
const { type, node } = binding.path;
switch (type) {
case 'FunctionDeclaration':
case 'ClassDeclaration':
case 'VariableDeclaration':
return true;
case 'VariableDeclarator':
const { init } = node;
if (t.isCallExpression(init) && init.callee.name === 'require') {
return false;
case 'FunctionDeclaration':
case 'ClassDeclaration':
case 'VariableDeclaration':
return true;
case 'VariableDeclarator': {
const { init } = node;
if (t.isCallExpression(init) && init.callee.name === 'require') {
return false;
}
return true;
}
return true;
default:
return false;
default:
return false;
}
}

const REGISTRATIONS = Symbol();
return {
visitor: {
ExportDefaultDeclaration(path, { file }) {
ExportDefaultDeclaration(path, { file }) {
// Default exports with names are going
// to be in scope anyway so no need to bother.
if (path.node.declaration.id) {
Expand All @@ -73,55 +74,57 @@ module.exports = function(args) {
t.toExpression(path.node.declaration);
path.insertBefore(
t.variableDeclaration('const', [
t.variableDeclarator(id, expression)
t.variableDeclarator(id, expression),
])
)
path.node.declaration = id;
);
path.node.declaration = id; // eslint-disable-line no-param-reassign

// It won't appear in scope.bindings
// so we'll manually remember it exists.
path.parent[REGISTRATIONS].push(
buildRegistration({
ID: id,
NAME: t.stringLiteral('default'),
FILENAME: t.stringLiteral(file.opts.filename)
FILENAME: t.stringLiteral(file.opts.filename),
})
);
},

Program: {
enter({ node, scope }, { file }) {
node[REGISTRATIONS] = [];
node[REGISTRATIONS] = []; // eslint-disable-line no-param-reassign

// Everything in the top level scope, when reasonable,
// is going to get tagged with __source.
for (let id in scope.bindings) {
/* eslint-disable guard-for-in,no-restricted-syntax */
for (const id in scope.bindings) {
const binding = scope.bindings[id];
if (shouldRegisterBinding(binding)) {
node[REGISTRATIONS].push(buildRegistration({
ID: binding.identifier,
NAME: t.stringLiteral(id),
FILENAME: t.stringLiteral(file.opts.filename)
FILENAME: t.stringLiteral(file.opts.filename),
}));
}
}
/* eslint-enable */
},

exit({ node, scope }) {
let registrations = node[REGISTRATIONS];
node[REGISTRATIONS] = null;
exit({ node }) {
const registrations = node[REGISTRATIONS];
node[REGISTRATIONS] = null; // eslint-disable-line no-param-reassign

// Inject the generated tagging code at the very end
// so that it is as minimally intrusive as possible.
node.body.push(buildSemi());
node.body.push(
buildTagger({
REGISTRATIONS: registrations
REGISTRATIONS: registrations,
})
);
node.body.push(buildSemi());
}
}
}
},
},
},
};
}
};
2 changes: 1 addition & 1 deletion src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ module.exports = function warnAboutIncorrectUsage(arg) {
'require("react-hot-loader").AppContainer.'
);
}
}
};

module.exports.AppContainer = AppContainer;
36 changes: 19 additions & 17 deletions src/patch.dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,16 @@ class ComponentMap {
get(type) {
if (this.wm) {
return this.wm.get(type);
} else {
const slot = this.getSlot(type);
for (let i = 0; i < slot.length; i++) {
if (slot[i].key === type) {
return slot[i].value;
}
}

const slot = this.getSlot(type);
for (let i = 0; i < slot.length; i++) {
if (slot[i].key === type) {
return slot[i].value;
}
}

return undefined;
}

set(type, value) {
Expand All @@ -52,15 +54,15 @@ class ComponentMap {
has(type) {
if (this.wm) {
return this.wm.has(type);
} else {
const slot = this.getSlot(type);
for (let i = 0; i < slot.length; i++) {
if (slot[i].key === type) {
return true;
}
}

const slot = this.getSlot(type);
for (let i = 0; i < slot.length; i++) {
if (slot[i].key === type) {
return true;
}
return false;
}
return false;
}
}

Expand All @@ -80,7 +82,7 @@ const hooks = {
if (typeof uniqueLocalName !== 'string' || typeof fileName !== 'string') {
return;
}
const id = fileName + '#' + uniqueLocalName;
const id = fileName + '#' + uniqueLocalName; // eslint-disable-line prefer-template
if (!idsByType.has(type) && hasCreatedElementsByType.has(type)) {
if (!didWarnAboutID[id]) {
didWarnAboutID[id] = true;
Expand Down Expand Up @@ -113,7 +115,7 @@ const hooks = {
didWarnAboutID = {};
hasCreatedElementsByType = new ComponentMap(useWeakMap);
idsByType = new ComponentMap(useWeakMap);
}
},
};

hooks.reset(typeof WeakMap === 'function');
Expand All @@ -127,12 +129,12 @@ function resolveType(type) {
hasCreatedElementsByType.set(type, true);

// When available, give proxy class to React instead of the real class.
var id = idsByType.get(type);
const id = idsByType.get(type);
if (!id) {
return type;
}

var proxy = proxiesByID[id];
const proxy = proxiesByID[id];
if (!proxy) {
return type;
}
Expand Down
2 changes: 2 additions & 0 deletions src/patch.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/* eslint-disable global-require */

'use strict';

if (process.env.NODE_ENV === 'production') {
Expand Down
10 changes: 5 additions & 5 deletions src/webpack/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,20 +48,20 @@ function transform(source, map) {
if (this.sourceMap === false) {
return this.callback(null, [
source,
appendText
appendText,
].join(separator));
}

if (!map) {
map = makeIdentitySourceMap(source, this.resourcePath);
map = makeIdentitySourceMap(source, this.resourcePath); // eslint-disable-line no-param-reassign
}
const node = new SourceNode(null, null, null, [
SourceNode.fromStringWithSourceMap(source, new SourceMapConsumer(map)),
new SourceNode(null, null, this.resourcePath, appendText)
new SourceNode(null, null, this.resourcePath, appendText),
]).join(separator);

const result = node.toStringWithSourceMap();
this.callback(null, result.code, result.map.toString());
};
return this.callback(null, result.code, result.map.toString());
}

module.exports = transform;
Loading