Skip to content

Updated Babel, Webpack, React, Eslint, etc to latest & Clean up linting errors#719

Open
ryank311 wants to merge 21 commits intogoatslacker:masterfrom
ryank311:master
Open

Updated Babel, Webpack, React, Eslint, etc to latest & Clean up linting errors#719
ryank311 wants to merge 21 commits intogoatslacker:masterfrom
ryank311:master

Conversation

@ryank311
Copy link
Copy Markdown

Hey, I upgraded most of the dependencies to latest versions and cleaned up all the linting errors across all the files. I tried to keep it as close to the original source where applicable, and if I wasn't sure on a rule, I just commented it out with /eslint-disable-line. All tests should pass with this PR and build appears to be good with the latest webpack.

-Ryan

@ryank311
Copy link
Copy Markdown
Author

Looks like there was an issue with the coverage plugin. Will look into that and ammend this PR once I fix.

@ryank311
Copy link
Copy Markdown
Author

I fixed the issue with the travis CLI. It was due to an updated dependency. I also found and fixed an issue with the babel test file due to the eslinting.

@@ -1,5 +1,5 @@
{
"presets": ["airbnb", "stage-0"],
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'd like to keep this as-is

"modules": true,
"jsx": true
},
"rules": {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

same here, can we revert this file to include only the single rule. We can perhaps add individual overrides per line.

no-param-reassign can be easily fixed by renaming all the reducers from obj to acc (acc is whitelisted as an acceptable param that can be reassigned)

"react/jsx-filename-extension": 0,
"react/forbid-prop-types": 0,
},
"parser": "babel-eslint"
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

don't think we need this, but we may need it for some stage-0 features. I'd happily .eslintignore anything that is stage0 though, since it's supposed to be experimental

@@ -1,5 +1,9 @@
# Changelog

## 1.0.0
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

oh no not yet.

"flux": "2.1.1",
"is-promise": "2.1.0",
"transmitter": "3.0.1"
"flux": "^3.1.3",
Copy link
Copy Markdown
Owner

@goatslacker goatslacker Nov 15, 2017

Choose a reason for hiding this comment

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

let's downgrade this back down to 2.1.1, the improvements past that version just bloat the library unnecessarily especially for what Alt is using the flux lib for. And the new add-ons in 3 aren't worth the bump.

"repository": {
"type": "git",
"url": "https://github.com/goatslacker/alt.git"
"url": "https://github.com/ryank311/alt.git"
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

??

"clean": "rimraf lib",
"coverage": "npm run transpile-cover && babel-node node_modules/.bin/istanbul cover node_modules/.bin/_mocha -- -u exports -R tap --require test/babel test",
"lint": "eslint src components",
"lint-fix": "eslint --fix src test scripts",
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Nice! 👍

neverFetchUsers: {
remote,
local: () => false,
local: () => { return false },
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

what's up with all these changes?

this.props.alt.stores.store.listen(state => this.setState(state))
}
componentWillMount() {
this.props.alt.stores.store.listen((state) => { return this.setState(state) })
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

weird spacing

@goatslacker
Copy link
Copy Markdown
Owner

Can you rebase so that the commits are split up into units of work?

  • webpack upgrade
  • babel upgrade
  • eslint fixes
  • eslint upgrades
  • test upgrades
  • other various upgrades
  • checked in build

We should actually remove the checked-in build or fix webpack so that it's more deterministic.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants