module: add support for abi stable module API #11975
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -180,6 +180,9 @@ static const char* trace_enabled_categories = nullptr; | |
| std::string icu_data_dir; // NOLINT(runtime/string) | ||
| #endif | ||
|
|
||
| // N-API is in experimental state, disabled by default. | ||
| bool load_napi_modules = false; | ||
|
|
||
| // used by C++ modules as well | ||
| bool no_deprecation = false; | ||
|
|
||
|
|
@@ -2463,15 +2466,25 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) { | |
| } | ||
| if (mp->nm_version != NODE_MODULE_VERSION) { | ||
| char errmsg[1024]; | ||
| if (mp->nm_version == -1) { | ||
| snprintf(errmsg, | ||
| sizeof(errmsg), | ||
| "The module '%s'" | ||
| "\nwas compiled against the ABI-stable Node.js API (N-API)." | ||
| "\nThis feature is experimental and must be enabled on the " | ||
| "\ncommand-line by adding --napi-modules.", | ||
| *filename); | ||
| } else { | ||
| snprintf(errmsg, | ||
| sizeof(errmsg), | ||
| "The module '%s'" | ||
| "\nwas compiled against a different Node.js version using" | ||
| "\nNODE_MODULE_VERSION %d. This version of Node.js requires" | ||
| "\nNODE_MODULE_VERSION %d. Please try re-compiling or " | ||
| "re-installing\nthe module (for instance, using `npm rebuild` " | ||
| "or `npm install`).", | ||
| *filename, mp->nm_version, NODE_MODULE_VERSION); | ||
| } | ||
|
|
||
| // NOTE: `mp` is allocated inside of the shared library's memory, calling | ||
| // `uv_dlclose` will deallocate it | ||
|
|
@@ -3535,6 +3548,7 @@ static void PrintHelp() { | |
| " --trace-deprecation show stack traces on deprecations\n" | ||
| " --throw-deprecation throw an exception on deprecations\n" | ||
| " --no-warnings silence all process warnings\n" | ||
| " --napi-modules load N-API modules\n" | ||
| " --trace-warnings show stack traces on process warnings\n" | ||
| " --redirect-warnings=path\n" | ||
| " write warnings to path instead of\n" | ||
|
|
@@ -3705,6 +3719,8 @@ static void ParseArgs(int* argc, | |
| force_repl = true; | ||
| } else if (strcmp(arg, "--no-deprecation") == 0) { | ||
| no_deprecation = true; | ||
| } else if (strcmp(arg, "--napi-modules") == 0) { | ||
| load_napi_modules = true; | ||
| } else if (strcmp(arg, "--no-warnings") == 0) { | ||
| no_process_warnings = true; | ||
| } else if (strcmp(arg, "--trace-warnings") == 0) { | ||
|
|
@@ -4473,6 +4489,11 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data, | |
| if (debug_enabled) | ||
| EnableDebug(&env); | ||
|
|
||
| if (load_napi_modules) { | ||
| ProcessEmitWarning(&env, "N-API is an experimental feature " | ||
| "and could change at any time."); | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. I’m not a fan of printing warnings like these… if the user opted into the feature, then they opted in because they know what they were asking for. There was a problem hiding this comment. CTC has specifically requested command-line enablement along with a warning message for features with experimental status. Tracing does the same thing. There was a problem hiding this comment. Is there a link somewhere to that decision? I don't quite understand the justification for a printed warning for a feature that can't even be used without a flag. If you're using the flag, you wanted the feature and are quite aware that it is experimental. Do any V8 features do this? There was a problem hiding this comment. VM Summit 3 meeting notes has this There was a problem hiding this comment. If consensus is that documentation of experimental status is sufficient and this warning message is not necessary, we can take it out. I don't recall anyone feeling very strongly about this at the VM summit. There was a problem hiding this comment. @jasongin I’d say take it out, and we can re-visit that in the unlikely case anybody complains. There was a problem hiding this comment. I think the main motivation was to ensure that anybody turning it on is fully aware of the status and to be consistent with what we have done for other experimental features. |
||
| { | ||
| SealHandleScope seal(isolate); | ||
| bool more; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should match the description in
doc/api/cli.mdfor consistency.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in e1ca374