src: implement constants and natives binding directly by joyeecheung · Pull Request #48186 · nodejs/node

joyeecheung · 2023-05-26T12:12:05Z

src: implement natives binding without special casing

This patch removes special case in the internal binding loader
for natives, and implements it using the builtins internal
binding. Internally we do not actually need the natives binding,
so implement it as a legacy wrapper instead.

src: implement constants binding directly

Instead of adding a special case for it in the internal binding
loader, just implement it as usual using a per-context property
initializer.

This patch removes special case in the internal binding loader for natives, and implements it using the builtins internal binding. Internally we do not actually need the natives binding, so implement it as a legacy wrapper instead.

nodejs-github-bot · 2023-05-26T12:12:09Z

Review requested:

@nodejs/loaders
@nodejs/modules
@nodejs/startup

joyeecheung · 2023-05-26T12:12:47Z

The original condition was already redundant, builtinIds actually contains both public and internal IDs, so I just removed it (as well as bootstrap_node which no longer exists)

Instead of adding a special case for it in the internal binding loader, just implement it as usual using a per-context property initializer.

nodejs-github-bot · 2023-05-31T14:37:15Z

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

nodejs-github-bot · 2023-06-01T15:31:58Z

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

nodejs-github-bot · 2023-06-06T09:23:33Z

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

nodejs-github-bot · 2023-06-07T13:38:25Z

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

nodejs-github-bot · 2023-06-08T08:44:25Z

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

This patch removes special case in the internal binding loader for natives, and implements it using the builtins internal binding. Internally we do not actually need the natives binding, so implement it as a legacy wrapper instead. PR-URL: #48186 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

Instead of adding a special case for it in the internal binding loader, just implement it as usual using a per-context property initializer. PR-URL: #48186 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

joyeecheung · 2023-06-09T13:43:56Z

Landed in 6a3403c...d102f18

This patch removes special case in the internal binding loader for natives, and implements it using the builtins internal binding. Internally we do not actually need the natives binding, so implement it as a legacy wrapper instead. PR-URL: #48186 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

Instead of adding a special case for it in the internal binding loader, just implement it as usual using a per-context property initializer. PR-URL: #48186 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

This patch removes special case in the internal binding loader for natives, and implements it using the builtins internal binding. Internally we do not actually need the natives binding, so implement it as a legacy wrapper instead. PR-URL: nodejs#48186 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

Instead of adding a special case for it in the internal binding loader, just implement it as usual using a per-context property initializer. PR-URL: nodejs#48186 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

This patch removes special case in the internal binding loader for natives, and implements it using the builtins internal binding. Internally we do not actually need the natives binding, so implement it as a legacy wrapper instead. PR-URL: nodejs#48186 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

Instead of adding a special case for it in the internal binding loader, just implement it as usual using a per-context property initializer. PR-URL: nodejs#48186 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

ruyadorno · 2023-08-29T21:55:44Z

hi @joyeecheung these commits are not landing cleanly on v18.x-staging and will need manual backport in case we want to land this in v18.

src: implement natives binding without special casing
f83abce
This patch removes special case in the internal binding loader for natives, and implements it using the builtins internal binding. Internally we do not actually need the natives binding, so implement it as a legacy wrapper instead.

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 May 26, 2023

joyeecheung commented May 26, 2023
View reviewed changes

ljharb reviewed May 26, 2023
View reviewed changes

Comment threadlib/internal/legacy/processbinding.js Outdated

anonrig reviewed May 26, 2023
View reviewed changes

Comment threadlib/internal/v8_prof_processor.js Outdated

src: implement constants binding directly
6f7a253
Instead of adding a special case for it in the internal binding loader, just implement it as usual using a per-context property initializer.

joyeecheung force-pushed the constants branch from 16cf719 to 6f7a253Compare May 31, 2023 14:36

fixup! src: implement natives binding without special casing
8461ac1

This was referenced Jun 2, 2023
CI Reliability 2023-06-02 nodejs/reliability#583
Open
CI Reliability 2023-06-03 nodejs/reliability#584
Open

jasnell approved these changes Jun 3, 2023
View reviewed changes

This was referenced Jun 4, 2023
CI Reliability 2023-06-04 nodejs/reliability#585
Open
CI Reliability 2023-06-05 nodejs/reliability#586
Open

legendecas approved these changes Jun 5, 2023
View reviewed changes

github-actionsBot mentioned this pull request Jun 6, 2023
CI Reliability 2023-06-06 nodejs/reliability#587
Open
20 tasks

github-actionsBot mentioned this pull request Jun 9, 2023
CI Reliability 2023-06-09 nodejs/reliability#588
Open
24 tasks

joyeecheung closed this Jun 9, 2023

This was referenced Jun 10, 2023
CI Reliability 2023-06-10 nodejs/reliability#589
Open
CI Reliability 2023-06-11 nodejs/reliability#590
Open
CI Reliability 2023-06-12 nodejs/reliability#591
Open

RafaelGSS mentioned this pull request Jul 3, 2023
v20.4.0 proposal #48643
Merged

ruyadorno added the backport-requested-v18.x label Aug 29, 2023

Navigation Menu

Uh oh!

Uh oh!

src: implement constants and natives binding directly - #48186

src: implement constants and natives binding directly#48186
joyeecheung wants to merge 3 commits into
nodejs:mainfrom
joyeecheung:constants

joyeecheung commented May 26, 2023

Uh oh!

nodejs-github-bot commented May 26, 2023

Uh oh!

joyeecheungMay 26, 2023•
edited
Loading

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented May 31, 2023

Uh oh!

nodejs-github-bot commented Jun 1, 2023

Uh oh!

nodejs-github-bot commented Jun 6, 2023

Uh oh!

nodejs-github-bot commented Jun 7, 2023

Uh oh!

nodejs-github-bot commented Jun 8, 2023

Uh oh!

joyeecheung commented Jun 9, 2023

Uh oh!

ruyadorno commented Aug 29, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Uh oh!

Uh oh!

Conversation

joyeecheung commented May 26, 2023

src: implement natives binding without special casing

src: implement constants binding directly

Uh oh!

nodejs-github-bot commented May 26, 2023

Uh oh!

joyeecheungMay 26, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented May 31, 2023

Uh oh!

nodejs-github-bot commented Jun 1, 2023

Uh oh!

nodejs-github-bot commented Jun 6, 2023

Uh oh!

nodejs-github-bot commented Jun 7, 2023

Uh oh!

nodejs-github-bot commented Jun 8, 2023

Uh oh!

joyeecheung commented Jun 9, 2023

Uh oh!

ruyadorno commented Aug 29, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

joyeecheungMay 26, 2023•
edited
Loading