Skip to content

replace lodash with just, replace webpack with rollup, transpile dependencies #332

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

Merged
merged 5 commits into from
Oct 19, 2018

Conversation

heygrady
Copy link
Contributor

@heygrady heygrady commented Oct 16, 2018

fixes #331

  • naively copy over the rollup config from redux for umd builds
  • preserve babel build for commonjs and esm
  • allow babel to parse node_modules (to ensure transpilation of dependencies)
  • replace lodash.curry with just-curry-it
  • replace lodash.camelcase with to-camel-case
  • upgraded to latest reduce-reducers (increased size)
  • upgraded every package to latest
filename before after
dist/redux-actions.js 110 KB 19KB
dist/redux-actions.min.js 20KB (7KB gzipped) 7KB (3KB gzipped)

Notes:

  • Upgrading to latest xo meant disabling a linting rule. I left a note in the code explaining why.

@codecov
Copy link

codecov bot commented Oct 16, 2018

Codecov Report

Merging #332 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #332   +/-   ##
=======================================
  Coverage   96.21%   96.21%           
=======================================
  Files          27       27           
  Lines         132      132           
  Branches       39       39           
=======================================
  Hits          127      127           
  Misses          4        4           
  Partials        1        1
Impacted Files Coverage Δ
src/utils/camelCase.js 100% <ø> (ø) ⬆️
src/utils/ownKeys.js 50% <ø> (ø) ⬆️
src/createCurriedAction.js 100% <ø> (ø) ⬆️
src/handleActions.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 51de389...8ecf5b9. Read the comment docs.

Copy link
Member

@timche timche left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! Looks good apart from a change in a unit test.

@@ -9,5 +9,5 @@ test('includes forward slashes in words', () => {
});

test('does nothing to an already camel-cased action type', () => {
expect(camelCase('myAction')).toBe('myAction');
expect(camelCase('myAction')).toBe('myaction');
Copy link
Member

Choose a reason for hiding this comment

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

Please fix this behaviour, because it shouldn't do anything to the string according to the spec.

@heygrady
Copy link
Contributor Author

Wondering if I should revert to the way redux-actions handled the commonjs and es builds before. Using rollup creates a single-file commonjs build, which has benefits but maybe be a jarring API change for users that want to import a file directly from lib.

@bboydflo
Copy link

It would be nice to merge this! Great work @heygrady

@timche
Copy link
Member

timche commented Oct 18, 2018

Wondering if I should revert to the way redux-actions handled the commonjs and es builds before. Using rollup creates a single-file commonjs build, which has benefits but maybe be a jarring API change for users that want to import a file directly from lib.

So do u think there any breaking changes?

@heygrady
Copy link
Contributor Author

heygrady commented Oct 18, 2018

No. But the previous build did separate files in the lib folder, like this:
https://unpkg.com/[email protected]/lib/

This build will have a single file.

With the new build you can still do import { handleActions } from 'redux-actions'. But you can’t do import camelCase from 'redux-actions/lib/utils/camelCase' like you could before.

@timche
Copy link
Member

timche commented Oct 18, 2018

Can we change this to the old behaviour? Because I see this as potentially breaking.

@timche
Copy link
Member

timche commented Oct 19, 2018

Thanks 👍

@timche timche merged commit ef5f9d8 into redux-utilities:master Oct 19, 2018
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.

Importing lodash accounts for more than 50% of this package
3 participants