Skip to content

Remove async dependency - #255

Closed
mscdex wants to merge 1 commit into
form-data:masterfrom
mscdex:remove-async-dep
Closed

mscdex wants to merge 1 commit into
form-data:masterfrom
mscdex:remove-async-dep

Conversation

@mscdex

@mscdex mscdex commented Sep 17, 2016 •

Copy link
Copy Markdown

The async module currently pulls in lodash, which contributes mostly to async's current 5.5MB size. This commit removes the async dependency and replaces the single usage with a simple implementation.

This is especially important for dependents like request, which are heavily depended upon, so reducing the space used will have a positive effect for all.

The async module currently pulls in lodash, which contributes to
async's current 5.5MB size. This commit removes the async dependency
and replaces the single usage with a simple implementation.
@review-ninja

Copy link
Copy Markdown

ReviewNinja

@coveralls

coveralls commented Sep 17, 2016 •

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.4%) to 95.918% when pulling aca3579 on mscdex:remove-async-dep into 158443d on form-data:master.

@mscdex

mscdex commented Sep 17, 2016

Copy link
Copy Markdown
Author

/cc @alexindigo

@alexindigo

Copy link
Copy Markdown
Member

@mscdex Thanks for the PR. I do agree with the premise, async is too bloated. Although I don't think that 'form-data` is in business of flow control. We should remove code from here, not add.
And such things as parallel execution of async tasks should be in the separate properly tested library.

I replaced async with asynckit, it has zero dependencies and it provides more obvious API – where you iterate over list of values with iterator function, rather than executing list of the same function with different scope.

@mscdex

mscdex commented Sep 17, 2016 •

Copy link
Copy Markdown
Author

@alexindigo IMHO this single usage is simple enough and doesn't warrant an unnecessary dependency. However, the main goal is to dump async. I'm assuming when you said you replaced it, you did so locally, as I don't see the replacement in the master branch?

@alexindigo

Copy link
Copy Markdown
Member

I created PR #258 and waiting for windows checks to pass. As soon as it's done. I'll merge and release new version. Thanks for the pushing it forward :)

@mscdex mscdex closed this Sep 17, 2016
@alexindigo

Copy link
Copy Markdown
Member

PS. I wanted to replace iteration over functions with iteration over values. And even single use case requires tests, and tests for flow control in form-data lib, didn't feel like at home.

@alexindigo

Copy link
Copy Markdown
Member

Published 2.0.0

@mscdex

mscdex commented Sep 17, 2016

Copy link
Copy Markdown
Author

Thanks for the quick responses!

alexindigo referenced this pull request Sep 17, 2016
- Dropped support for node@0.10
- Cleaned up code and tools.
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.

4 participants