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.

Sign up for GitHub

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

Jump to bottom

Remove dependency on is-buffer #1816

Merged
merged 6 commits into from Nov 18, 2019
Merged

Remove dependency on is-buffer #1816

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
43461ff
Update utils.js
Chalarangelo Oct 1, 2018
d483bd4
Remove dependency on is-buffer from package.json
Chalarangelo Oct 1, 2018
9a6c538
Update utils.js
Chalarangelo Oct 1, 2018
325e581
Merge branch 'master' into master
yasuf Nov 9, 2019
1f8566e
Merge branch 'master' into master
yasuf Nov 16, 2019
ef01eef
Update isX.spec.js
Chalarangelo Nov 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 11 additions & 1 deletion lib/utils.js
View file
Open in desktop
@@ -1,7 +1,6 @@
'use strict';

var bind = require('./helpers/bind');
var isBuffer = require('is-buffer');

/*global toString:true*/

Expand All @@ -19,6 +18,17 @@ function isArray(val) {
return toString.call(val) === '[object Array]';
}


/**
* Determine if a value is a Buffer
*
* @param {Object} val The value to test
* @returns {boolean} True if value is a Buffer, otherwise false
*/
function isBuffer(val) {
return ![undefined, null].includes(val) && val.constructor === Buffer;
Copy link
Contributor

Choose a reason for hiding this comment

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

consider val != null here if the goal is to be equivalent to val !== null && val !== undefined

Copy link
Contributor

Choose a reason for hiding this comment

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

also consider checking typeof Buffer !== "undefined" so that we don't get strict mode errors related to the Buffer variable not being defined (browser tests should catch this?)

Copy link

@Mechazawa Mechazawa Oct 30, 2018 •
edited

Choose a reason for hiding this comment

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

just do this

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.

Why not simplify with Buffer.isBuffer ?
If the package is installed via npm, we can assume it's running in Node and has Buffer defined.
Otherwise, it gets polyfilled during Webpack builds for browsers.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's even used in other places of the code, like here: https://github.com/axios/axios/blob/master/lib/adapters/http.js#L41

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nevermind- I tried it and it injects the polyfill for the whole Buffer module. Your solution works best.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, are you going to merge it? Do you want me to do anything else?

}

/**
* Determine if a value is an ArrayBuffer
*
Expand Down
3 changes: 1 addition & 2 deletions package.json
View file
Open in desktop
Expand Up @@ -73,8 +73,7 @@
},
"typings": "./index.d.ts",
"dependencies": {
"follow-redirects": "1.5.10",
"is-buffer": "^2.0.2"
"follow-redirects": "^1.4.1"
},
"bundlesize": [
{
Expand Down
6 changes: 6 additions & 0 deletions test/specs/utils/isX.spec.js
View file
Open in desktop
Expand Up @@ -7,6 +7,12 @@ describe('utils::isX', function () {
expect(utils.isArray({length: 5})).toEqual(false);
});

it('should validate Buffer', function () {
expect(utils.isBuffer(Buffer.from('a'))).toEqual(true);
expect(utils.isBuffer(null)).toEqual(false);
expect(utils.isBuffer(undefined)).toEqual(false);
});

it('should validate ArrayBuffer', function () {
expect(utils.isArrayBuffer(new ArrayBuffer(2))).toEqual(true);
expect(utils.isArrayBuffer({})).toEqual(false);
Expand Down