src: handle no support for external buffers

[legendecas]

Fixes #1257.

• When NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED introduced in node-api: handle no support for external buffers node#45181 is defined, hides the methods that can create external buffers.
• Introduce Napi::Buffer::NewOrCopy to create buffer from external buffers that are compatible with runtimes like electron conveniently.

[lovell]

Thank you for adding this feature. It looks like the approach in this PR can (conditionally) remove the presence of Buffer<T>::New() from the API at compile time. Is this intentional? If so, please can this be considered a breaking, semver major change.

[legendecas]

@lovell it is intentional and NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED is following the definition in nodejs/node#45181. I don't think it is a breaking change, since user codes will not be affected if the newly introduced macro NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED is not defined.

[mhdawson]

Suggested change

	[external buffer][] is not supported, allocates a new `Napi::Buffer` object and
	[external buffer][] is not supported by the underlying runtime, allocates a new `Napi::Buffer` object and

[mhdawson]

LGTM other than the suggested wording change to the docs

[mhdawson]

@legendecas looks like compilation failed on windows:

  buffer.cc
D:\a\node-addon-api\node-addon-api\napi-inl.h(2504,17): error C2065: 'napi_no_external_buffers_allowed': undeclared identifier [D:\a\node-addon-api\node-addon-api\test\build\binding_noexcept_maybe.vcxproj]
D:\a\node-addon-api\node-addon-api\napi-inl.h(2500): message : while compiling class template member function 'Napi::Buffer<uint16_t> Napi::Buffer<uint16_t>::NewOrCopy(napi_env,T *,size_t)' [D:\a\node-addon-api\node-addon-api\test\build\binding_noexcept_maybe.vcxproj]
          with
          [
              T=uint16_t
          ]
D:\a\node-addon-api\node-addon-api\test\buffer.cc(153): message : see reference to function template instantiation 'Napi::Buffer<uint16_t> Napi::Buffer<uint16_t>::NewOrCopy(napi_env,T *,size_t)' being compiled [D:\a\node-addon-api\node-addon-api\test\build\binding_noexcept_maybe.vcxproj]
          with
          [
              T=uint16_t
          ]
D:\a\node-addon-api\node-addon-api\test\buffer.cc(29): message : see reference to class template instantiation 'Napi::Buffer<uint16_t>' being compiled [D:\a\node-addon-api\node-addon-api\test\build\binding_noexcept_maybe.vcxproj]
D:\a\node-addon-api\node-addon-api\napi-inl.h(2529,17): error C2065: 'napi_no_external_buffers_allowed': undeclared identifier [D:\a\node-addon-api\node-addon-api\test\build\binding_noexcept_maybe.vcxproj]
D:\a\node-addon-api\node-addon-api\test\buffer.cc(180): message : see reference to function template instantiation 'Napi::Buffer<uint16_t> Napi::Buffer<uint16_t>::NewOrCopy<`anonymous-namespace'::CreateOrCopyExternalBufferWithFinalize::<lambda_ed832a77ce45243ca6d5fe088f290640>>(napi_env,T *,size_t,Finalizer)' being compiled [D:\a\node-addon-api\node-addon-api\test\build\binding_noexcept_maybe.vcxproj]
          with
          [
              T=uint16_t,
              Finalizer=`anonymous-namespace'::CreateOrCopyExternalBufferWithFinali

[Julusian]

It looks like when NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED is set, that Buffer::NewOrCopy is being defined, with the implementation attempting to use napi_create_external_buffer which is missing because of the NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED flag.

When NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED either Buffer::NewOrCopy should also be disabled, or the implementation should skip calling napi_create_external_buffer and always do the copy?

Ideally I would like to be able to use Buffer::NewOrCopy which is able to do napi_create_external_buffer, but to also have the offending Buffer::New methods removed. That way I can't accidentally use Buffer::New, but I can still get the benefits of Buffer::NewOrCopy when the runtime allows. I don't know how this could be implemented though

[legendecas]

@Julusian thanks for the suggestion! I'll update the PR to include a test to verify Buffer::NewOrCopy is still usable when NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED is defined.

[legendecas]

Seems like the test async_progress_worker is constantly failing on windows-2019, v16.x: #1278 (comment)

[mhdawson]

I assume its the templates that prevent us from having one function with all the parameters and the others calling that one with dummy/null parameters, right?

[legendecas]

Not really. When the template parameter Hint is set, the Finalizer must accept a third parameter Hint too. However, we allow Finalizer to be like void operator()(Env env, T* data). So we have to distinguish if Hint is present.

[mhdawson]

LGTM on updated version with one question

[legendecas]

@mhdawson is this still looking good to you with the replied question?

If so, I think we've discussed about the failure of Node.js CI Windows Platform / test (16.x, x86, windows-2019) (pull_request) and we can suppress it at the moment and land this PR.

[mhdawson]

@legendecas LGTM
Landed as b16c762