Skip to content
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

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

Merged
merged 1 commit into from
Sep 18, 2023
Merged

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

merged 1 commit into from
Sep 18, 2023

Conversation

Fdawgs
Copy link
Contributor

@Fdawgs Fdawgs commented Sep 12, 2023

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.

@github-actions
Copy link
Contributor

No linked issues found. Please add the corresponding issues in the pull request description.
Use GitHub automation to close the issue when a PR is merged

@simoneb
Copy link
Member

simoneb commented Sep 18, 2023

Thanks, I didn't know this performance improvement existed. Have you come across e.g. a eslint setting so enforce this?

@Fdawgs
Copy link
Contributor Author

Fdawgs commented Sep 18, 2023

Thanks, I didn't know this performance improvement existed. Have you come across e.g. a eslint setting so enforce this?

Only https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/prefer-node-protocol.md so far!

@simoneb
Copy link
Member

simoneb commented Sep 18, 2023

Interesting, if it is really part of the recommended configuration, which I believe we use in all our packages, I would have expected to see that warning before, but I never saw it

@Fdawgs
Copy link
Contributor Author

Fdawgs commented Sep 18, 2023

Interesting, if it is really part of the recommended configuration, which I believe we use in all our packages, I would have expected to see that warning before, but I never saw it

Oh sorry, it's part of a third party plugin, not the main eslint recommended ones.

@simoneb
Copy link
Member

simoneb commented Sep 18, 2023

ah gotcha, well thanks for mentioning nonetheless!

@simoneb simoneb merged commit f836ce1 into nearform:master Sep 18, 2023
5 of 7 checks passed
@Fdawgs Fdawgs deleted the perf/builtins branch September 18, 2023 14:34
@github-actions github-actions bot mentioned this pull request Sep 20, 2023
@wyozi
Copy link

wyozi commented Jul 31, 2024

this breaks webpack: Module build failed: UnhandledSchemeError: Reading from "node:crypto" is not handled by plugins (Unhandled scheme). Webpack supports "data:" and "file:" URIs by default. You may need an additional plugin to handle "node:" URIs.

@simoneb
Copy link
Member

simoneb commented Jul 31, 2024

@wyozi please open a new issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants