n-api: implement napi_run_script

[gabrielschulhof]

Fixes: nodejs/abi-stable-node#51

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

Affected core subsystem(s)

n-api

[TimothyGu]

Make it clear that this is in UTF-8? Or just take a string napi_value instead?

[gabrielschulhof]

Maybe it'd be easier to take a napi_value instead.

[addaleax]

… encoded as UTF-8 and null-terminated?

[addaleax]

don’t we technically need to free() this?

[TimothyGu]

Pointer star goes with the type.

[addaleax]

We’re inconsistent about that in this file, and I think this can pass as “idiomatic for C code”

[TimothyGu]

ditto

[TimothyGu]

Alphabetical order

[gabrielschulhof]

Switched script parameter to napi_value, made sure that the script ->IsString(), and added a test to make sure it throws if it isn't.

[TimothyGu]

In the future we might want to have an equivalent for V8's Script or UnboundScript, but this LGTM and should suffice for now.

[jasnell]

Please avoid the use of you in the docs.

N-API allows the execution of a JavaScript string using the underlying JavaScript engine.

[mhdawson]

LGTM once comment from @jasnell is addressed

[gabrielschulhof]

@jasnell I have reworded the sentence.

[jasnell]

thank you!

[BridgeAR]

CI https://ci.nodejs.org/job/node-test-pull-request/10009/

[gabrielschulhof]

SmartOS re-run: https://ci.nodejs.org/job/node-test-commit-smartos/11353/

[gabrielschulhof]

ARM re-run: https://ci.nodejs.org/job/node-test-commit-arm/11989/

[gabrielschulhof]

Landed in 61e9ba1.

[digitalinfinity]

@gabrielschulhof sorry, missed reviewing this PR. @MSLaguana and I were looking at it for implementation in node-chakracore and we had a question- if someone was debugging their script and a script executed from this API was on the stack- what is the "filename" they expect to see? Dynamic code or something equivalent? Should there be a way of modules identifying themselves to the debugger? Or should these frames be considered "library code" and hidden from the debugger?

[gabrielschulhof]

@digitalinfinity maybe we should treat it like an eval. Chrome developer tools opens a new tab and calls it "VM<number>" where number is probably some internal counter or something like that. It paints the tab a different colour. In fact, I ran `node --inspect-brk test/addons-napi/test_general/testNapiRun.js` and when I stepped into `addon.testNapiRun()` it behaved exactly like `eval()`.

…

On Fri, Sep 15, 2017 at 2:49 AM, Hitesh Kanwathirtha < ***@***.***> wrote: @gabrielschulhof <https://github.com/gabrielschulhof> sorry, missed reviewing this PR. @MSLaguana <https://github.com/mslaguana> and I were looking at it for implementation in node-chakracore and we had a question- if someone was debugging their script and a script executed from this API was on the stack- what is the "filename" they expect to see? Dynamic code or something equivalent? Should there be a way of modules identifying themselves to the debugger? Or should these frames be considered "library code" and hidden from the debugger? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15216 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA7k0etVgsKC-XXzJmDMQLQoyJOln7O1ks5sibt6gaJpZM4POMQh> .