Skip to content

Commit

Permalink
src: disambiguate terms used to refer to builtins and addons
Browse files Browse the repository at this point in the history
The term "native module" dates back to some of the oldest code
in the code base. Within the context of Node.js core it usually
refers to modules that are native to Node.js (e.g. fs, http),
but it can cause confusion for people who don't work on this
part of the code base, as "native module" can also refer to
native addons - which is even the case in some of the API
docs and error messages.

This patch tries to make the usage of these terms more consistent.
Now within the context of Node.js core:

- JavaScript scripts that are built-in to Node.js are now referred
  to as "built-in(s)". If they are available as modules,
  they can also be referred to as "built-in module(s)".
- Dynamically-linked shared objects that are loaded into
  the Node.js processes are referred to as "addons".

We will try to avoid using the term "native modules" because it could
be ambiguous.

Changes in this patch:

File names:
- node_native_module.h -> node_builtins.h,
- node_native_module.cc -> node_builtins.cc

C++ binding names:
- `native_module` -> `builtins`

`node::Environment`:
- `native_modules_without_cache` -> `builtins_without_cache`
- `native_modules_with_cache` -> `builtins_with_cache`
- `native_modules_in_snapshot` -> `builtins_in_cache`
- `native_module_require` -> `builtin_module_require`

`node::EnvSerializeInfo`:
- `native_modules` -> `builtins

`node::native_module::NativeModuleLoader`:
- `native_module` namespace -> `builtins` namespace
- `NativeModuleLoader` -> `BuiltinLoader`
- `NativeModuleRecordMap` -> `BuiltinSourceMap`
- `NativeModuleCacheMap` -> `BuiltinCodeCacheMap`
- `ModuleIds` -> `BuiltinIds`
- `ModuleCategories` -> `BuiltinCategories`
- `LoadBuiltinModuleSource` -> `LoadBuiltinSource`

`loader.js`:
- `NativeModule` -> `BuiltinModule` (the `NativeModule` name used in
  `process.moduleLoadList` is kept for compatibility)

And other clarifications in the documentation and comments.

PR-URL: #44135
Fixes: #44036
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
  • Loading branch information
joyeecheung authored and legendecas committed Aug 8, 2022
1 parent fa22337 commit 472edc7
Show file tree
Hide file tree
Showing 42 changed files with 309 additions and 341 deletions.
2 changes: 1 addition & 1 deletion .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@

/benchmark/misc/startup.js @nodejs/startup
/src/node.cc @nodejs/startup
/src/node_native_module* @nodejs/startup
/src/node_builtins* @nodejs/startup
/src/node_snapshot* @nodejs/startup
/lib/internal/bootstrap/* @nodejs/startup
/tools/snapshot/* @nodejs/startup
Expand Down
4 changes: 2 additions & 2 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -1996,7 +1996,7 @@ changes:

Type: Compile-time

Certain versions of `node::MakeCallback` APIs available to native modules are
Certain versions of `node::MakeCallback` APIs available to native addons are
deprecated. Please use the versions of the API that accept an `async_context`
parameter.

Expand Down Expand Up @@ -2746,7 +2746,7 @@ changes:
Type: Documentation-only

The `node:repl` module exports a `_builtinLibs` property that contains an array
with native modules. It was incomplete so far and instead it's better to rely
of built-in modules. It was incomplete so far and instead it's better to rely
upon `require('node:module').builtinModules`.

### DEP0143: `Transform._transformState`
Expand Down
4 changes: 2 additions & 2 deletions doc/api/modules.md
Original file line number Diff line number Diff line change
Expand Up @@ -707,8 +707,8 @@ This does not apply to [native addons][], for which reloading will result in an
error.

Adding or replacing entries is also possible. This cache is checked before
native modules and if a name matching a native module is added to the cache,
only `node:`-prefixed require calls are going to receive the native module.
built-in modules and if a name matching a built-in module is added to the cache,
only `node:`-prefixed require calls are going to receive the built-in module.
Use with care!

<!-- eslint-disable node-core/no-duplicate-requires -->
Expand Down
8 changes: 4 additions & 4 deletions doc/api/n-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,9 @@ GitHub projects using CMake.js.
#### prebuildify

[prebuildify][] is a tool based on node-gyp. The advantage of prebuildify is
that the built binaries are bundled with the native module when it's
that the built binaries are bundled with the native addon when it's
uploaded to npm. The binaries are downloaded from npm and are immediately
available to the module user when the native module is installed.
available to the module user when the native addon is installed.

## Usage

Expand Down Expand Up @@ -1384,7 +1384,7 @@ callback throws an exception with no way to recover.

### Fatal errors

In the event of an unrecoverable error in a native module, a fatal error can be
In the event of an unrecoverable error in a native addon, a fatal error can be
thrown to immediately terminate the process.

#### `napi_fatal_error`
Expand Down Expand Up @@ -5724,7 +5724,7 @@ Returns `napi_ok` if the API succeeded.

This function gives V8 an indication of the amount of externally allocated
memory that is kept alive by JavaScript objects (i.e. a JavaScript object
that points to its own memory allocated by a native module). Registering
that points to its own memory allocated by a native addon). Registering
externally allocated memory will trigger global garbage collections more
often than it would otherwise.

Expand Down
2 changes: 1 addition & 1 deletion doc/contributing/adding-new-napi-api.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Contributing a new API to Node-API

Node-API is the next-generation ABI-stable API for native modules.
Node-API is the next-generation ABI-stable API for native addons.
While improving the API surface is encouraged and welcomed, the following are
a set of principles and guidelines to keep in mind while adding a new
Node-API.
Expand Down
4 changes: 2 additions & 2 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ const { openSync, closeSync, readSync } = require('fs');
const { inspect } = require('internal/util/inspect');
const { isPromise, isRegExp } = require('internal/util/types');
const { EOL } = require('internal/constants');
const { NativeModule } = require('internal/bootstrap/loaders');
const { BuiltinModule } = require('internal/bootstrap/loaders');
const { isError } = require('internal/util');

const errorCache = new SafeMap();
Expand Down Expand Up @@ -303,7 +303,7 @@ function getErrMessage(message, fn) {

// Skip Node.js modules!
if (StringPrototypeStartsWith(filename, 'node:') &&
NativeModule.exists(StringPrototypeSlice(filename, 5))) {
BuiltinModule.exists(StringPrototypeSlice(filename, 5))) {
errorCache.set(identifier, undefined);
return;
}
Expand Down
30 changes: 16 additions & 14 deletions lib/internal/bootstrap/loaders.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
// and have their nm_flags set to NM_F_INTERNAL.
//
// Internal JavaScript module loader:
// - NativeModule: a minimal module system used to load the JavaScript core
// - BuiltinModule: a minimal module system used to load the JavaScript core
// modules found in lib/**/*.js and deps/**/*.js. All core modules are
// compiled into the node binary via node_javascript.cc generated by js2c.py,
// so they can be loaded faster without the cost of I/O. This class makes the
Expand Down Expand Up @@ -180,9 +180,9 @@ let internalBinding;

const loaderId = 'internal/bootstrap/loaders';
const {
moduleIds,
builtinIds,
compileFunction
} = internalBinding('native_module');
} = internalBinding('builtins');

const getOwn = (target, property, receiver) => {
return ObjectPrototypeHasOwnProperty(target, property) ?
Expand All @@ -195,13 +195,13 @@ const getOwn = (target, property, receiver) => {
* Be careful not to expose this to user land unless --expose-internals is
* used, in which case there is no compatibility guarantee about this class.
*/
class NativeModule {
class BuiltinModule {
/**
* A map from the module IDs to the module instances.
* @type {Map<string, NativeModule>}
* @type {Map<string, BuiltinModule>}
*/
static map = new SafeMap(
ArrayPrototypeMap(moduleIds, (id) => [id, new NativeModule(id)])
ArrayPrototypeMap(builtinIds, (id) => [id, new BuiltinModule(id)])
);

constructor(id) {
Expand All @@ -216,7 +216,7 @@ class NativeModule {
this.loading = false;

// The following properties are used by the ESM implementation and only
// initialized when the native module is loaded by users.
// initialized when the built-in module is loaded by users.
/**
* The C++ ModuleWrap binding used to interface with the ESM implementation.
* @type {ModuleWrap|undefined}
Expand All @@ -232,7 +232,7 @@ class NativeModule {
// To be called during pre-execution when --expose-internals is on.
// Enables the user-land module loader to access internal modules.
static exposeInternals() {
for (const { 0: id, 1: mod } of NativeModule.map) {
for (const { 0: id, 1: mod } of BuiltinModule.map) {
// Do not expose this to user land even with --expose-internals.
if (id !== loaderId) {
mod.canBeRequiredByUsers = true;
Expand All @@ -241,11 +241,11 @@ class NativeModule {
}

static exists(id) {
return NativeModule.map.has(id);
return BuiltinModule.map.has(id);
}

static canBeRequiredByUsers(id) {
const mod = NativeModule.map.get(id);
const mod = BuiltinModule.map.get(id);
return mod && mod.canBeRequiredByUsers;
}

Expand Down Expand Up @@ -327,14 +327,16 @@ class NativeModule {

const fn = compileFunction(id);
// Arguments must match the parameters specified in
// NativeModuleLoader::LookupAndCompile().
// BuiltinLoader::LookupAndCompile().
fn(this.exports, requireFn, this, process, internalBinding, primordials);

this.loaded = true;
} finally {
this.loading = false;
}

// "NativeModule" is a legacy name of "BuiltinModule". We keep it
// here to avoid breaking users who parse process.moduleLoadList.
ArrayPrototypePush(moduleLoadList, `NativeModule ${id}`);
return this.exports;
}
Expand All @@ -344,7 +346,7 @@ class NativeModule {
// written in CommonJS style.
const loaderExports = {
internalBinding,
NativeModule,
BuiltinModule,
require: nativeModuleRequire
};

Expand All @@ -353,7 +355,7 @@ function nativeModuleRequire(id) {
return loaderExports;
}

const mod = NativeModule.map.get(id);
const mod = BuiltinModule.map.get(id);
// Can't load the internal errors module from here, have to use a raw error.
// eslint-disable-next-line no-restricted-syntax
if (!mod) throw new TypeError(`Missing internal module '${id}'`);
Expand All @@ -363,7 +365,7 @@ function nativeModuleRequire(id) {
// Allow internal modules from dependencies to require
// other modules from dependencies by providing fallbacks.
function requireWithFallbackInDeps(request) {
if (!NativeModule.map.has(request)) {
if (!BuiltinModule.map.has(request)) {
request = `internal/deps/${request}`;
}
return nativeModuleRequire(request);
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
// modules to use.
// - `lib/internal/bootstrap/loaders.js`: to setup internal binding and
// module loaders, including `process.binding()`, `process._linkedBinding()`,
// `internalBinding()` and `NativeModule`.
// `internalBinding()` and `BuiltinModule`.
//
// This file is run to bootstrap both the main thread and the worker threads.
// After this file is run, certain properties are setup according to the
Expand Down Expand Up @@ -89,7 +89,7 @@ process.domain = null;
process._exiting = false;

// process.config is serialized config.gypi
const nativeModule = internalBinding('native_module');
const nativeModule = internalBinding('builtins');

// TODO(@jasnell): Once this has gone through one full major
// release cycle, remove the Proxy and setter and update the
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/debugger/inspect_repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ function extractFunctionName(description) {
}

const {
moduleIds: PUBLIC_BUILTINS,
} = internalBinding('native_module');
builtinIds: PUBLIC_BUILTINS,
} = internalBinding('builtins');
const NATIVES = internalBinding('natives');
function isNativeUrl(url) {
url = RegExpPrototypeSymbolReplace(/\.js$/, url, '');
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/main/mksnapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const {
} = primordials;

const binding = internalBinding('mksnapshot');
const { NativeModule } = require('internal/bootstrap/loaders');
const { BuiltinModule } = require('internal/bootstrap/loaders');
const {
compileSerializeMain,
} = binding;
Expand Down Expand Up @@ -92,7 +92,7 @@ function supportedInUserSnapshot(id) {
}

function requireForUserSnapshot(id) {
if (!NativeModule.canBeRequiredByUsers(id)) {
if (!BuiltinModule.canBeRequiredByUsers(id)) {
// eslint-disable-next-line no-restricted-syntax
const err = new Error(
`Cannot find module '${id}'. `
Expand Down
12 changes: 6 additions & 6 deletions lib/internal/modules/cjs/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const {
ERR_MANIFEST_DEPENDENCY_MISSING,
ERR_UNKNOWN_BUILTIN_MODULE
} = require('internal/errors').codes;
const { NativeModule } = require('internal/bootstrap/loaders');
const { BuiltinModule } = require('internal/bootstrap/loaders');

const { validateString } = require('internal/validators');
const path = require('path');
Expand All @@ -42,10 +42,10 @@ const cjsConditions = new SafeSet([
...userConditions,
]);

function loadNativeModule(filename, request) {
const mod = NativeModule.map.get(filename);
function loadBuiltinModule(filename, request) {
const mod = BuiltinModule.map.get(filename);
if (mod?.canBeRequiredByUsers) {
debug('load native module %s', request);
debug('load built-in module %s', request);
// compileForPublicLoader() throws if mod.canBeRequiredByUsers is false:
mod.compileForPublicLoader();
return mod;
Expand Down Expand Up @@ -73,7 +73,7 @@ function makeRequireFunction(mod, redirects) {
const href = destination.href;
if (destination.protocol === 'node:') {
const specifier = destination.pathname;
const mod = loadNativeModule(specifier, href);
const mod = loadBuiltinModule(specifier, href);
if (mod && mod.canBeRequiredByUsers) {
return mod.exports;
}
Expand Down Expand Up @@ -230,7 +230,7 @@ module.exports = {
addBuiltinLibsToObject,
cjsConditions,
hasEsmSyntax,
loadNativeModule,
loadBuiltinModule,
makeRequireFunction,
normalizeReferrerURL,
stripBOM,
Expand Down
Loading

0 comments on commit 472edc7

Please sign in to comment.