Revert "vm: add importModuleDynamically option to compileFunction"

[mcollina]

This reverts commit 74c393d.

Fixes: #33166
See: #32985
See: #31860

Checklist

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

[mcollina]

@SimenB this should likely fix Babel and Jest.

[SimenB]

I hope it can re-land later as I wanna use the code this reverts. 😀 But it does seem prudent to revert for now.
I wonder if the problem points to some other deep-lying issue that this change just exposed, though. If building using a different compiler changes things it seems likely this JS change is just a symptom and not the cause.

[nodejs-github-bot]

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

[benjamingr]

Rubber stamp LGTM due to revert + severity.

(Plus a non-rubber-stamp-lgtm on the delta like the md changes)

[mcollina]

I wonder if the problem points to some other deep-lying issue that this change just exposed, though. If building using a different compiler changes things it seems likely this JS change is just a symptom and not the cause.

I literally have no clue on how to debug that. I know there is something in vs2019 that is affecting stdout, but it's really very hard to track. cc @ronag #32487

[targos]

@mcollina is #33166 also reproducible on nightlies from master? If not, I think we should actually only revert on v14.x.

[mcollina]

It fails also on the nightlies

[devsnek]

I still don't get why we consider this commit the issue and not vs2019

[mcollina]

I still don't get why we consider this commit the issue and not vs2019

@devsnek because this commit triggers the problem. Node v14 is largely broken on one of the supported Operating Systems, and it is important that we ship a fix sooner rather than later. However I have got no clue on how to approach the vs2019 issue and it might take an unbounded amount of time to fix it. We currently have no strategy around this problem.

I'm currently pinging some connections in Microsoft to see if they can help, but I fear it might take a long time to get to a conclusion as this problem is very ephemeral.

[devsnek]

@mcollina I mean why are we saying "build without this commit" instead of "build without vs2019". This issue could easily manifest itself in other ways (I think jasnell mentioned stream timing bugs?) so it seems like we should use a better compiler.

[mcollina]

I do not know how long would it take for the @nodejs/build to change the compiler we are using to build our Node.js releases. I suspect it's a significant amount of effort. I do not think we can wait fixing this bug on v14.

[addaleax]

I’m a bit worried here too … I get that this is a severe problem, but without understanding why this commit causes it – it almost certainly only exposes a pre-existing bug – there’s a decent chance that we’ll do some other thing that re-exposes it in the future without knowing.

[jasnell]

Yep. I consider this revert to be temporary to fix the acute breakage but the revert itself is not the solution.

[mhdawson]

LGTM

[nodejs-github-bot]

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

[mcollina]

Landed in 2d5d773