module: add changes that should have been in 18394

[devsnek]

these changes make it so that the new behavior of ModuleWrap::Link will be used by vm.Module and enforced by tests for vm.Module, and as we don't specifically test our internal bindings i think this will be good enough.

See: #18394

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

Affected core subsystem(s)

src, vm

[devsnek]

cc @iamstolis

[BridgeAR]

@devsnek is it possible to add a test for this?

[devsnek]

@BridgeAR i'm not sure exactly how, we don't expose ModuleWrap itself and generally we test implementations not their native backing right? i originally couldn't figure out how to test this because i wasn't sure how to insert a super long job into the node loader, but its usage in vm.Module ensures the behavior works, which is how i caught that bug with settings the elements of the promise array.

[BridgeAR]

Hm ok. If someone has an idea: I guess it would be nice to add a test just to make sure.

[iamstolis]

Looks good to me, thanks for the fix.

[TimothyGu]

I should've caught this sooner... LGTM with tests.

[bnoordhuis]

...and a better commit log. What changes? Why?

[devsnek]

afaik there's no way to test internalBinding modules, if anyone has an idea on how to do that please let me know

[targos]

afaik there's no way to test internalBinding modules, if anyone has an idea on how to do that please let me know

You can export the binding from an internal/ module

[devsnek]

@targos that seems pretty messy, maybe we can provide internalBinding to certain tests by folder or something similar? (perhaps tests/internal, as a separate pr, although i don't want to block this pr, i'm very conflicted rn, i'll add it for this pr and then look into removing it later)

[TimothyGu]

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

[TimothyGu]

Landed in b0f114d.

[MylesBorins]

This should come with #17560 when it is backported

[devsnek]

this should be directly backported with #18394 not #17560 or the loader will stop working. i'm not sure what to do about the changes to the vm module file

[MylesBorins]

Should this be backported to v8.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.