Tracking Issue: process.binding to internalBinding

[jasnell]

Moving from process.binding() to internalBinding() will be a significant effort. There are hundreds of process.binding() uses inside core and many many userland references. It's going to take a while to convert everything.

TODO

• inspector (this one is quite non-trivial... will require additional thought)

Done

• async_wrap #22469
• buffer #22370
• cares_wrap #22474
• crypto #22426
• config #23400
• constants #23663
• contextify #22419
• fs #22478
• fs_event_wrap #22480
• http2#22328
• http_parser #22329
• icu #23234
• js_stream #22239
• natives #23663
• os #25087
• performance #22029
• pipe_wrap #22482
• process_wrap #22479
• serdes #22161
• signal_wrap #22290
• spawn_sync #22260
• stream_wrap
• tcp_wrap #22432
• tls_wrap #22429
• trace_events #22159
• tty_wrap #22477
• udp_wrap #22475
• url #22204
• util #22675
• uv #22163
• v8 #22288
• zlib (removing process.binding('zlib') breaks npm) #23307

[devsnek]

updated to include all usage of NODE_BUILTIN_MODULE_CONTEXT_AWARE

[jasnell]

I've got uv in progress now

[devsnek]

@ChALkeR would it be possible to get info about the relative usage of all of these? i'm assuming there are some like serdes we can kill right away and others like util that will take longer.

[jasnell]

I tried doing zlib but it broke npm.
I tried doing config but there's a buffer test that isn't feasible if config is moved. Planning to refactor

[maclover7]

Just double checking, should all migration PRs should be marked as semver-major?

[jasnell]

Unfortunately that's likely the best thing.

[jasnell]

For some of this it's likely going to be quite tricky to have a proper migration strategy in place given that moving something over to internalBinding() means that the process.binding() version just stops working. We're going to have to take it very slow and work on migrating the bits with the smallest immediate impact and working up from there.

[pratik0509]

I would also like to contribute here. Can you please give me some heads up?

[jmannanc]

Hey,

I've been trying to move buffer to internalBinding to try and help out and I think I've gotten close but make -j4 test keeps giving these errors: https://hastebin.com/ecucikeraj.cs

You can see the branch that I'm working on here. Any help on maybe how to fix these errors/why they are happening would be very much appreciated 😄. Sorry for the bother if my changes are incorrect or this is not allowed.

[danbev]

@sneakyfish5 In the tests, I think we should be using require('internal/test/binding'). And also make sure that we have the --expose-internals flag. For example:

// Flags: --expose-internals
...
const { internalBinding } = require('internal/test/binding');

Could you double check the changes to the tests to verify that require('internal/bootstrap/loaders'); is not being used, which would produce the error you reported.

[targos]

Is all of this going to be backported to v10.x-staging (with all bindings in the whitelist)?

[jasnell]

@targos ... just to be extra safe, these are all semver-major even with the whitelist.

[targos]

@jasnell OK, no problem. Then these should probably be labeled semver-major: #22370 #22480 #22482

[Deleted user]

Why is this even being discussed? Why does it matter? Please clarify 😕 @jasnell

[starkwang]

Now the inspector module is the only one left to be converted. Should we open a new issue to track it ? @jasnell

[BeniCheni]

Hi, @starkwang. it appears that #24931 took care of inspector.

[jasnell]

I think we can close this now.