Skip to content

buffer: do not emit deprecation notice on Buffer.of #19682

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

Closed
TimothyGu wants to merge 3 commits into from
+22 −0
Closed

buffer: do not emit deprecation notice on Buffer.of #19682

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0eaff92
buffer: do not emit deprecation notice on Buffer.of
TimothyGu Mar 29, 2018
9491d3b
fixup! buffer: do not emit deprecation notice on Buffer.of
TimothyGu Mar 29, 2018
4be9962
fixup! buffer: do not emit deprecation notice on Buffer.of
TimothyGu Mar 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions lib/buffer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,17 @@ Buffer.from = function from(value, encodingOrOffset, length) {
);
};

// Identical to the built-in %TypedArray%.of(), but avoids using the deprecated
// Buffer() constructor.
//
// Refs: https://tc39.github.io/ecma262/#sec-%typedarray%.of
Buffer.of = (...items) => {
Copy link
Copy Markdown
Member

@ChALkeR ChALkeR Mar 29, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

This changes the name of the function. It should probably be named 'of'.

Copy link
Copy Markdown
Member Author

@TimothyGu TimothyGu Mar 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Grrr, I always thought this kind of assignment names the function automatically. Fixed.

Copy link
Copy Markdown
Member

@ChALkeR ChALkeR Mar 30, 2018

Choose a reason for hiding this comment

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

They are named in cases like { of: () => {} } and const of = () => {}, but not in x.of = () => {}.

const newObj = createUnsafeBuffer(items.length);
for (var k = 0; k < items.length; k++)
newObj[k] = items[k];
return newObj;
Copy link
Copy Markdown
Member

@ChALkeR ChALkeR Mar 29, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Line-to-line code duplication of fromArrayLike.
These four lines could be replaced with return fromArrayLike(items).

Copy link
Copy Markdown
Member Author

@TimothyGu TimothyGu Mar 30, 2018

Choose a reason for hiding this comment

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

No. fromArrayLike uses allocate() while this function uses createUnsafeBuffer(). The former may be pooled, which is not desirable.

};

Object.setPrototypeOf(Buffer, Uint8Array);

// The 'assertSize' method will remove itself from the callstack when an error
Expand Down
8 changes: 8 additions & 0 deletions test/parallel/test-buffer-of-no-deprecation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Flags: --pending-deprecation --no-warnings
'use strict';

const common = require('../common');

process.on('warning', common.mustNotCall());
Copy link
Copy Markdown
Contributor

@cjihrig cjihrig Mar 30, 2018

Choose a reason for hiding this comment

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

Should the --no-warnings flag be dropped?

Copy link
Copy Markdown
Member

@jasnell jasnell Apr 1, 2018

Choose a reason for hiding this comment

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

No, the --no-warnings just stops the console output. The process.on('warning') event will still emit.


Buffer.of(0, 1);