GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — [v16.x backport] src: disambiguate terms used to refer to builtins and addons by mhdawson · Pull Request #45663 · nodejs/node

Conversation

mhdawson commented Nov 28, 2022

Copy link

Copy Markdown

Member

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 [email protected]
Reviewed-By: Matteo Collina [email protected]
Reviewed-By: Michael Dawson [email protected]
Reviewed-By: Richard Lau [email protected]
Reviewed-By: Jiawen Geng [email protected]
Reviewed-By: Chengzhong Wu [email protected]
Reviewed-By: Mohammed Keyvanzadeh [email protected]
Reviewed-By: Tobias Nießen [email protected]
Reviewed-By: Jan Krems [email protected]

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          src: disambiguate terms used to refer to builtins and addons

bd8e25b

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 <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Jan Krems <[email protected]>

nodejs-github-bot commented Nov 28, 2022

Copy link

Copy Markdown

Collaborator

Review requested:

@nodejs/crypto
@nodejs/gyp
@nodejs/modules
@nodejs/node-api
@nodejs/startup
@nodejs/tsc

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

nodejs-github-bot added lib / src

Issues and PRs involving general changes in the lib/ or src/ directories.

needs-ci

PRs that need a full CI run.

v16.x labels

Nov 28, 2022

mhdawson commented Nov 28, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member Author

Did this backport to unlock backporting of #44376

This was not an easy backport, many conflicts and there are several PRs that are marked as don't land on 16.x on which this depends. This meant that additional/different files like node_builtins_env.cc needed to be updated. Quite a lot of manual work so needs a good review.

One key question which we should confirm is that changing the name of the builtin is SemVer patch as indicated on the original PR. I assume it is but @joyeecheung could you confirm?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mhdawson mentioned this pull request

Nov 28, 2022

src: add support for externally shared js builtins #44376

Closed

mhdawson added a commit to mhdawson/io.js that referenced this pull request

Nov 28, 2022


          src: cleanup on disambiguating native modules

1f7e71f

Found while backporting
nodejs#45663

Fixup one rename missed

Signed-off-by: Michael Dawson <[email protected]>

mhdawson mentioned this pull request

Nov 28, 2022

src: cleanup on disambiguating native modules #45665

Closed

mscdex changed the title ~~src: disambiguate terms used to refer to builtins and addons~~ [v16.x backport] src: disambiguate terms used to refer to builtins and addons

Nov 29, 2022

mhdawson added a commit that referenced this pull request

Dec 2, 2022


          src: cleanup on disambiguating native modules

22c645d

Found while backporting
#45663

Fixup one rename missed

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #45665
Reviewed-By: Daeyeon Jeong <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>

richardlau added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Dec 7, 2022

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Dec 7, 2022

nodejs-github-bot commented Dec 7, 2022

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

nodejs-github-bot commented Dec 7, 2022

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

richardlau approved these changes

Dec 8, 2022

View reviewed changes

richardlau pushed a commit that referenced this pull request

Dec 8, 2022


          src: disambiguate terms used to refer to builtins and addons

fbc52e5

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
Backport-PR-URL: #45663
Fixes: #44036
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Jan Krems <[email protected]>

richardlau commented Dec 8, 2022

Copy link

Copy Markdown

Member

Landed in fbc52e5.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

richardlau closed this

Dec 8, 2022

targos pushed a commit that referenced this pull request

Dec 12, 2022


          src: cleanup on disambiguating native modules

79edf25

Found while backporting
#45663

Fixup one rename missed

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #45665
Reviewed-By: Daeyeon Jeong <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>

danielleadams pushed a commit that referenced this pull request

Dec 30, 2022


          src: cleanup on disambiguating native modules

d4155a2

Found while backporting
#45663

Fixup one rename missed

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #45665
Reviewed-By: Daeyeon Jeong <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>

guangwong pushed a commit to noslate-project/node that referenced this pull request

Jan 3, 2023


          src: disambiguate terms used to refer to builtins and addons

c7d6925

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/node#44135
Backport-PR-URL: nodejs/node#45663
Fixes: nodejs/node#44036
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Jan Krems <[email protected]>

guangwong pushed a commit to noslate-project/node that referenced this pull request

Jan 3, 2023


          src: disambiguate terms used to refer to builtins and addons

0f7e779

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/node#44135
Backport-PR-URL: nodejs/node#45663
Fixes: nodejs/node#44036
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Jan Krems <[email protected]>

danielleadams pushed a commit that referenced this pull request

Jan 3, 2023


          src: cleanup on disambiguating native modules

29f2dab

Found while backporting
#45663

Fixup one rename missed

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #45665
Reviewed-By: Daeyeon Jeong <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>

danielleadams pushed a commit that referenced this pull request

Jan 5, 2023


          src: cleanup on disambiguating native modules

edcb31d

Found while backporting
#45663

Fixup one rename missed

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #45665
Reviewed-By: Daeyeon Jeong <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>

Reviewers

richardlau

richardlau approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

lib / src

Issues and PRs involving general changes in the lib/ or src/ directories.

needs-ci

PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

mhdawson commented Nov 28, 2022

Uh oh!

nodejs-github-bot commented Nov 28, 2022

Uh oh!

mhdawson commented Nov 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Dec 7, 2022

Uh oh!

nodejs-github-bot commented Dec 7, 2022

Uh oh!

richardlau commented Dec 8, 2022

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mhdawson commented Nov 28, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.