Update dependencies and fixes warnings #86
Conversation
Manually tried to fix all ESLint errors
.babelrc
Outdated
| } | ||
| } | ||
| "presets": ["env", "react"], | ||
| "plugins": ["transform-object-rest-spread"] |
There was a problem hiding this comment.
I think babel-preset-env includes transform-object-rest-spread at this point (it's stage 4)
Using "env" without setting explicit browser versions should result in this configuration being used:
0.5%, last 2 versions, Firefox ESR, not dead
Is this what we want/need?
There was a problem hiding this comment.
Hmm the repo even points out that it's now included in the main Babel repo: https://github.com/babel/babel-preset-env
There was a problem hiding this comment.
Although that's Babel 7.0, which is a whole different beast :)
package.json
Outdated
| "cookie": "^0.3.1", | ||
| "cross-env": "^4.0.0", | ||
| "dependency-graph": "^0.5.0", | ||
| "es6-promise": "^4.1.0", | ||
| "es6-promise": "^4.2.5", |
There was a problem hiding this comment.
I don't know this project, just wondering what we support that requires the es6-promise polyfill
There was a problem hiding this comment.
Yep, that can go I think!
| dash_html_components==0.11.0rc5 | ||
| dash==0.18.3 | ||
| dash==0.28.0 |
There was a problem hiding this comment.
I think it would be better if it was unlocked with >=0.18.3 so we don't have to update the version and can see if an update to dash break the renderer. Same with dcc, but html need the locked rc version for py37 compat.
There was a problem hiding this comment.
Not dash>=0.28.0 then, in that case?
|
|
||
| function appLifecycle(state=APP_STATES('STARTED'), action) { | ||
| function appLifecycle(state = getAppState('STARTED'), action) { |
There was a problem hiding this comment.
Why was this changed ? Is it part of the prettier update ?
I am not sure they should be renamed as they are meant to be constants and in regular redux pattern the ACTIONS are always capitalized.
As a side note, I don't think the ACTIONS/APP_STATES should be functions, it only send an error if there is no action defined, that is only useful when developing since dispatching invalid actions is not something that should happen in a production app. You also don't get autocompletion in IDE with the function approach, meaning it is easier to make typos and get that error.
Not related, but I find the three different constants files a bit confusing.
There was a problem hiding this comment.
This was changed because it threw a linting error, stating that functions shouldn't be capitalized unless it's a constructor. I agree with you that they shouldn't be functions to begin with, though! I was planning on doing a bigger refactor once this is merged, as part of the loading states PR I'm working on. For now, I would like to merge this (if it won't cause issues) so that I can move on with bigger refactor stuff - this at least stops the linting errors.
This here updates some dependencies (and adds Prettier support), mostly similar to the changes in dash-core-components. I had to modify the code here and there because it gave linting errors. Looks like the tests still work however and manual QA I did didn't gave me any problems.
I also added Jest + Enzyme at some point, but decided to remove that for now, hence the commits specifying that.
edit:
closes #87