-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Drop Node 12 in tests and add Node 16 #7935
Conversation
Benchmark ResultsKitchen Sink 🚨
Timings
Cold BundlesNo bundles found, this is probably a failed build... Cached BundlesNo bundles found, this is probably a failed build... React HackerNews ✅
Timings
Cold Bundles
Cached BundlesNo bundle changes detected. AtlasKit Editor 🚨
Timings
Cold BundlesNo bundles found, this is probably a failed build... Cached BundlesNo bundles found, this is probably a failed build... Three.js ✅
Timings
Cold Bundles
Cached BundlesNo bundle changes detected. |
This makes sense to me, along with a plan to drop Node 12 as a compile target at a specific date in the future. We have apps that still use Node 12 and have Node's EOL as a motivating factor to update, so I wouldn't be surprised if this is the case elsewhere too. As grace period, maybe some time around October when Node cuts major releases anyway? We can probably drop Node 12 from CI now or once we get a plan to support 16 there (re: the |
importModuleDynamically: (specifier, referrer) => | ||
entry(specifier, referrer), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The vm module used in the integration tests requires a change for Node 16. The only item regarding vm in the changelog
A prime example of how adding a paramater to a callback is a breaking change...
@devongovett You'll have to merge this because it removed some required CI jobs (and probably also change which checks are required in the settings). |
Node 12 is EOL at the end of April.
We should add Node 16 to CI, but also the the number of jobs should stay the same to not make flakeyness even worse. So either
vm
module used in the integration tests requires a change for Node 16. The only item regarding vm in the changelog is vm: add support for import assertions in dynamic imports nodejs/node#40249