Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: disambiguate terms used to refer to builtins and addons #44135

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -41,10 +41,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 @@ -72,7 +72,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 @@ -229,7 +229,7 @@ module.exports = {
addBuiltinLibsToObject,
cjsConditions,
hasEsmSyntax,
loadNativeModule,
loadBuiltinModule,
makeRequireFunction,
normalizeReferrerURL,
stripBOM,
Expand Down
Loading