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

Use node: prefix to bypass require.cache call for builtins #824

Merged
merged 1 commit into from
Sep 25, 2023
Merged

Use node: prefix to bypass require.cache call for builtins #824

merged 1 commit into from
Sep 25, 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.

@Balearica Balearica merged commit 2a27a0e into naptha:master Sep 25, 2023
6 checks passed
@Balearica
Copy link
Member

Thanks for your contribution. Merged.

@Fdawgs Fdawgs deleted the perf/builtins branch September 25, 2023 15:28
@Balearica
Copy link
Member

This change ended up included in v4.1.3, however I ended up reverting it shortly after and releasing a v4.1.4. Unfortunately, it turns out that support for the node: prefix was introduced towards the end of Node.js v14 (minor release 18 of 21), meaning that is breaks Tesseract.js for most versions of Node.js 14. This was regrettably not caught by the automated testing as that uses the latest version of Node.js 14.

As Tesseract.js v4 was advertised as supporting Node.js v14, dropping support is a breaking change, so therefore semantic versioning rules dictate this cannot be implemented in another v4 version. While I am releasing v5 shortly, I'm still opting not to include this for now as I've learned that a surprising number of users use old versions of software. Can add this back in the eventual Tesseract.js v6, once Node.js v14 is sufficiently in the rear view that we won't get (many) complaints dropping support.

@Fdawgs
Copy link
Contributor Author

Fdawgs commented Sep 28, 2023

No worries, thanks for the update @Balearica! And thanks for continuing to maintain this!

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.

2 participants