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

joyeecheung · 2022-08-04T18:35:27Z

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.

Fixes: #44036

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.

nodejs-github-bot · 2022-08-04T18:35:31Z

JakobJingleheimer

Thanks for clarifying & cleaning this!

mcollina

lgtm

mhdawson

LGTM. This may make backports harder to some LTS lines, but I think the additional clarity is worth it.

legendecas

👍

VoltrexMaster · 2022-08-05T06:10:49Z

The code-owners linter seems to still be looking for src/node_native_module.cc instead of the renamed src/node_builtins.cc.

nodejs-github-bot · 2022-08-08T11:49:12Z

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

tniessen

RSLGTM.

jkrems

Thanks for doing this!

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>

legendecas · 2022-08-08T17:37:13Z

Landed in 472edc7

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>

danielleadams · 2022-08-16T02:51:43Z

This commit has merge conflicts after pulling #38905 out of v18.x, so I'm adding a backport-blocked to this commit

ruyadorno · 2022-08-23T02:40:07Z

Removing the backport-blocked-v18.x label after landing #38905 again to the staging branch.

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>

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: nodejs#44135 Fixes: nodejs#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>

This allows the misc/startup benchmark to run again after the renaming of the C++ `native_module` to `builtins` PR-URL: #44727 Refs: #44135 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>

juanarbol · 2022-10-02T23:21:24Z

After multiple tries landing this in the v16.x branch, I still have some issues; I will label this as "backport-requested-v16.x"

This allows the misc/startup benchmark to run again after the renaming of the C++ `native_module` to `builtins` PR-URL: #44727 Refs: #44135 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>

nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Aug 4, 2022

This comment was marked as outdated.

Sign in to view

JakobJingleheimer approved these changes Aug 4, 2022

View changes

mcollina approved these changes Aug 4, 2022

View changes

mhdawson approved these changes Aug 4, 2022

View changes

richardlau approved these changes Aug 4, 2022

View changes

gengjiawen approved these changes Aug 5, 2022

View changes

legendecas approved these changes Aug 5, 2022

View changes

VoltrexMaster approved these changes Aug 5, 2022

View changes

fixup! src: disambiguate terms used to refer to builtins and addons

7c3ae1e

tniessen approved these changes Aug 8, 2022

View changes

jkrems approved these changes Aug 8, 2022

View changes

legendecas closed this Aug 8, 2022

danielleadams mentioned this pull request Aug 16, 2022

v18.8.0 release proposal #44246

Closed

danielleadams added backport-blocked-v18.x PRs that should land on the v18.x-staging branch but are blocked by another PR's pending backport. and removed backport-blocked-v18.x PRs that should land on the v18.x-staging branch but are blocked by another PR's pending backport. labels Aug 16, 2022

ruyadorno removed the backport-blocked-v18.x PRs that should land on the v18.x-staging branch but are blocked by another PR's pending backport. label Aug 23, 2022

ruyadorno mentioned this pull request Aug 23, 2022

v18.8.0 release proposal #44353

Merged

aduh95 mentioned this pull request Sep 20, 2022

benchmark: fix startup benchmark #44727

Merged

juanarbol added the backport-requested-v16.x PRs awaiting manual backport to the v16.x-staging branch. label Oct 2, 2022

juanarbol mentioned this pull request Oct 3, 2022

bootstrap: generate bootstrapper arguments in BuiltinLoader #44488

Merged

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

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

joyeecheung commented Aug 4, 2022 •

edited

nodejs-github-bot commented Aug 4, 2022

This comment was marked as outdated.

JakobJingleheimer left a comment

mcollina left a comment

mhdawson left a comment

legendecas left a comment

VoltrexMaster commented Aug 5, 2022 •

edited

nodejs-github-bot commented Aug 8, 2022

tniessen left a comment

jkrems left a comment

legendecas commented Aug 8, 2022

danielleadams commented Aug 16, 2022

ruyadorno commented Aug 23, 2022 •

edited

juanarbol commented Oct 2, 2022

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

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

Conversation

joyeecheung commented Aug 4, 2022 • edited

nodejs-github-bot commented Aug 4, 2022

This comment was marked as outdated.

JakobJingleheimer left a comment

mcollina left a comment

mhdawson left a comment

legendecas left a comment

VoltrexMaster commented Aug 5, 2022 • edited

nodejs-github-bot commented Aug 8, 2022

tniessen left a comment

jkrems left a comment

legendecas commented Aug 8, 2022

danielleadams commented Aug 16, 2022

ruyadorno commented Aug 23, 2022 • edited

juanarbol commented Oct 2, 2022

joyeecheung commented Aug 4, 2022 •

edited

VoltrexMaster commented Aug 5, 2022 •

edited

ruyadorno commented Aug 23, 2022 •

edited