-
Notifications
You must be signed in to change notification settings - Fork 296
Use ES5 compatible camelcase library. #309
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
Conversation
@@ -7,5 +7,5 @@ export default type => | |||
? camelCase(type) | |||
: type | |||
.split(namespacer) | |||
.map(camelCase) | |||
.map(part => camelCase(part)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
due to camelCase
accepting multiple arguments, and map
providing the index
as a second argument, and entire array as 3rd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure about this? We had it like this before as well with camelCase
from lodash
.
webpack.config.js
Outdated
@@ -12,6 +12,7 @@ module.exports = { | |||
library: 'ReduxActions', | |||
libraryTarget: 'umd' | |||
}, | |||
devtool: 'sourcemaps', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes the UMD non-minified bundle not be eval sourcemaps.
Codecov Report
@@ Coverage Diff @@
## master #309 +/- ##
========================================
+ Coverage 95.96% 96% +0.03%
========================================
Files 24 24
Lines 124 125 +1
Branches 37 37
========================================
+ Hits 119 120 +1
Misses 4 4
Partials 1 1
Continue to review full report at Codecov.
|
@@ -61,6 +61,10 @@ acorn@^5.0.0, acorn@^5.3.0, acorn@^5.5.0: | |||
version "5.5.3" | |||
resolved "https://registry.yarnpkg.com/acorn/-/acorn-5.5.3.tgz#f473dd47e0277a08e28e9bec5aeeb04751f0b8c9" | |||
|
|||
acorn@~5.4.0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't usually use yarn
so I have no clue if I updated this properly (just letting yarn add
do it's thing)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything good.
package.json
Outdated
"build:umd": "cross-env NODE_ENV=development webpack", | ||
"build:umd:min": "cross-env NODE_ENV=production webpack", | ||
"build:umd": "cross-env NODE_ENV=development webpack && es-check es5 dist/redux-actions.js", | ||
"build:umd:min": "cross-env NODE_ENV=production webpack && es-check es5 dist/redux-actions.min.js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I confirmed that this failed before my change (NOTE: it passed on the unminified before due to eval sourcemaps obscuring the actual code which is why I added the sourcemap option below)
Thanks a lot for this PR! Would it be possible if we can remove |
Want me to just remove devtool and only test against the minified version with es-check? Or just remove es-check entirely (and devtool). Totally up to you. Just wanted to add some automated tests around this for future safety. |
Yeah, just replace the package. |
Released in v2.5.1: https://github.com/redux-utilities/redux-actions/releases/tag/v2.5.1 Sorry for the inconveniences caused. |
Thank you so much @timche! |
Fixes #308