Skip to content
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

Modernized typechecks #2255

Closed
wants to merge 4 commits into from
Closed

Conversation

fed135
Copy link
Collaborator

@fed135 fed135 commented Jun 28, 2019

Description

Modernizes typecheck functions in utils in order to reduce dependency count and reduce bundled dist sizes.

Results

Dist

before:
4740B

after:
4595B

total:
145B (~3.1%)

NPM

before:
83.6 kB

after:
82.1 kB

total:
1.5kB (~1.2%)

Notes

I tried a number of things on the side, this addresses some low-hanging fruits, but I think that the size of the bundle can be cut down significantly more. Let me know if that something that would be interesting.

Copy link
Collaborator

@yasuf yasuf left a comment

Choose a reason for hiding this comment

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

I like this change, nice decrease on the build size

function isArrayBuffer(val) {
return toString.call(val) === '[object ArrayBuffer]';
function isBuffer(val) {
return val && val.constructor && typeof val.constructor.isBuffer === 'function' && val.constructor.isBuffer(val);
Copy link
Collaborator

Choose a reason for hiding this comment

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

The nice thing about the dependency was that it had tests, do you mind adding some tests?

@jasonsaayman
Copy link
Member

Hi,

Thank you for the contribution. This includes the compiled files as well as not having tests, closing for now, please re-submit if you fix the mentioned issues.

Thanks

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.

None yet

3 participants