Use Buffer.alloc() instead of deprecated new Buffer() in copy-file-sync

[manidlou]

Since new Buffer() is deprecated, https://nodejs.org/api/buffer.html#buffer_new_buffer_size, I updated lib/copy-sync/copy-file-sync.js to use Buffer.alloc() instead of deprecated new Buffer().

[coveralls]

Coverage decreased (-5.9%) to 79.521% when pulling 785705b on copy-file-sync-update-Buffer-alloc into e771770 on master.

[manidlou]

Why coverage decreased by -5.9%?!! 😳. The only thing that is changed is just a word!

Edit

IDK, maybe related to lemurheavy/coveralls-public#463.

[jprichardson]

Great idea! The engine field in package.json must be set to at least 4.5.0 as Buffer.alloc didn't exist before it. Please add that change 😄

[RyanZim]

@jprichardson Wasn't aware of that. Isn't that a breaking change?

[jprichardson]

@jprichardson Wasn't aware of that. Isn't that a breaking change?

Not strictly - it's more or less tightening the rules, as fs-extra still supports Node v4. Just not pre v4.5.0 - to be extra safe, I'm okay if we do another major bump but may not be necessary.

[coveralls]

Coverage decreased (-5.9%) to 79.521% when pulling 6bff133 on copy-file-sync-update-Buffer-alloc into e771770 on master.

[coveralls]

Coverage remained the same at 79.521% when pulling 5597bd5 on copy-file-sync-update-Buffer-alloc into e06440b on master.

[manidlou]

I rebased this on top of master.

[jprichardson]

Thanks @manidlou!

[manidlou]

My pleasure 😄!

[dr-dimitru]

Hello @manidlou ,

I guess I'm nine days late... But why don't we keep backward compatibility and use Buffer.alloc only when it's exists, otherwise fallback to new Buffer constructor?

[RyanZim]

@dr-dimitru Actually, this change was reverted as a hotfix for v2.1.1. I'm thinking about some kind of fallback solution for future releases.

[dr-dimitru]

Hi @RyanZim ,

Thank you, good to know. And I'm better to read CHANGELOG more carefully.
I believe same should be done for move-sync.

This is how we use it on our packages (for .from() method):

if (typeof Buffer.from === 'function') {
  try {
    // Node 4.4.* Buffer.from already exists, but throws error
    buff = Buffer.from(body, 'base64');
  } catch (_error) {
    buff = new Buffer(body, 'base64');
  }
} else {
  buff = new Buffer(body, 'base64');
}

I can send a PR if you don't mind and agree with suggested code

[manidlou]

Sorry, I've been so busy. I also agree with providing some kind of fallback solution for this.

[RyanZim]

@dr-dimitru I also fixed moveSync in v2.1.1.

Thanks for telling me about Buffer.from in v4.4.x; wasn't aware of that.

I'm gonna try to open an issue later tonight to get some insight/discussion on this.

[dr-dimitru]

@RyanZim Thank you, keep me updated.

The code-sample above is just an example, same applies to Buffer.alloc too, and other new methods