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 'includes' API, fix CI build failure #2574

Merged
merged 4 commits into from Dec 9, 2019
Merged

Remove 'includes' API, fix CI build failure #2574

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
06533df
Remove 'includes' API, fix CI build failure
Alanscut Nov 27, 2019
33c8895
request change
Alanscut Dec 6, 2019
ba16c9a
request changes
Alanscut Dec 6, 2019
c0c7d05
Merge branch 'master' into CI-Error-includes
Dec 9, 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
3 changes: 2 additions & 1 deletion lib/utils.js
View file
Open in desktop
Expand Up @@ -26,7 +26,8 @@ function isArray(val) {
* @returns {boolean} True if value is a Buffer, otherwise false
*/
function isBuffer(val) {
return ![undefined, null].includes(val) && val.constructor === Buffer;
if (val === null || val === void 0 || val.constructor === null || val.constructor === void 0 ) return false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's the difference between using void 0 and undefined?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

undefined is not a reserved word in javascript, so it can be rewritten. void 0 is stricter.

Copy link
Collaborator

@yasuf yasuf Dec 5, 2019 •
edited

Choose a reason for hiding this comment

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

we rely on type of x === 'undefined' throughout the file, might be better to be consistent, also are there cases where the constructor can be null? could it be sufficient to just check !isUndefined(val.constructor), also we can just use our own helper? isUndefined, what do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree to use isUndefined, and I suggest to replace all the other undefined judgement with our own helper function isUndefined, if you agree, then I will submit a PR.

if constructor has been overwriten with null (extreme paticular case), then typeof val.constructor.isBuffer === 'function' will report an error.

return typeof val.constructor.isBuffer === 'function' && val.constructor.isBuffer(val);
chinesedfan marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down