search

nodejs / webidl-napi

A WebIDL-to-N-API compiler
MIT License
10 stars 6 forks source link

headers: complete separation #10

Closed gabrielschulhof closed 4 years ago

gabrielschulhof commented 4 years ago

Finish moving implementations out of webidl-napi.h into webidl-napi-inl.h.

Signed-off-by: @gabrielschulhof

gabrielschulhof commented 4 years ago

I recommend reviewing while ignoring whitespace.

cerisano commented 4 years ago

I can confirm that webidl-napi is now building with Node 10. Just want to be sure that webidl-napi does not enforce any nodejs dependencies in the generated runtime addon. There was a similar separation performed in node-addon-api in Node 12. I am thinking that this PR is intended to ensure that, even in Node 10 (which required an experimental flag to be set). Is it still the case that generating a nodejs-independent addon with webidl-napi requires Node 12 as a min req?

gabrielschulhof commented 4 years ago

@cerisano the code generated by this tool does in no way depend on Node.js. The generated code includes node_api.h if you build with -DBUILDING_NODE_EXTENSION because in Node.js v10.x js_native_api.h does not exist. Even so, the code the tool generates relies exclusively on APIs declared in js_native_api.h, so the resulting binary shared object has no dependencies on Node.js.

This PR is merely a housekeeping one. I'm following the pattern we used in node-addon-api, which is that webidl-napi.h does all the declarations, and webidl-napi-inl.h has all the inline implementations.

gabrielschulhof commented 4 years ago

Landed in a472ca75dc70a83eca25bd1ef16e04c3193c01be.