perf: use node: prefix to bypass require.cache call for builtins

[Fdawgs]

Summary

Introduced as part of v16.0.0 and v14.18.0, this allows redundant require.cache calls to be bypassed for builtin modules, saving a few yoctoseconds.

See https://nodejs.org/api/modules.html#core-modules and discussion in nodejs/node repo regarding why require.cache calls are redundant for builtins.

Test plan

N/A

[netlify]

✅ Deploy Preview for jestjs ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	ef5566d
🔍 Latest deploy log	https://app.netlify.com/sites/jestjs/deploys/65049ec1dfc5fe000840bb49
😎 Deploy Preview	https://deploy-preview-14515--jestjs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[SimenB]

Is there some eslint rule we can use which forces (and autofixes) this?

[SimenB]

Type error will probably be fixed by updating from node 14 types to node 16 (which will be done for Jest 30 - I'll start landing breaking changes next week)

[SimenB]

There is https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/prefer-node-protocol.md at least

[Fdawgs]

There is https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/prefer-node-protocol.md at least

Probably something that should be rolled into https://github.com/eslint-community/eslint-plugin-n, are you happy for me to add the unicorn plugin in in the meantime with that rule enabled?

[SimenB]

I've landed the unicorn plugin on main now, so feel free to activate the rule 🙂

Lots of test failures here. Also, we have some tests that explicitly use one or the other to ensure Jest's own resolver handles both correctly. So in some files we'll need to disable the rule

[Fdawgs]

Cheers @SimenB, should be able to revisit this in the next few days.

[SimenB]

Great, thank you!

[Fdawgs]

Also, we have some tests that explicitly use one or the other to ensure Jest's own resolver handles both correctly. So in some files we'll need to disable the rule

@SimenB could you point me to a file that does this as an example, please?

[SimenB]

Might be it just matters here

jest/e2e/native-esm/__tests__/native-esm.test.js

Lines 8 to 15 in 6d2632a

	import dns from 'dns';
	// the point here is that it's the node core module
	// eslint-disable-next-line no-restricted-imports
	import {readFileSync} from 'fs';
	import {createRequire} from 'module';
	import prefixDns from 'node:dns';
	import {dirname, resolve} from 'path';
	import {fileURLToPath} from 'url';

I doubt unit tests of isCoreModule is affected.

You might hit #14297 tho

[Fdawgs]

Going to be doing this piecemeal, package-by-package, just so I don't miss anything, so apologies in advance!

[SimenB]

No need to apologise! I'm super grateful you're taking the time to do this 🙂

[SimenB]

New version of @types/node has landed on master, which should resolve the TS error

[Fdawgs]

Thanks @SimenB, will revisit shortly. Was hitting #14297 at most points though. :(

[SimenB]

Yeah, it might make sense to fix that before changing our own references...

[SimenB]

fair 😀

[Fdawgs]

fair 😀

Disappearing for a bit and probably won't be back until Feb next year. This will no doubt be mighty out of date by then so thought i'd just nuke it and return to it when I can.

[SimenB]

Enjoy your time off! 😃

I genuinely meant that it's fair to abandon this - it's very chore-y and onerous 🙂 I realize it might've come off as sarcastic 😅

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.