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 #407

Merged
merged 2 commits into from Sep 8, 2023
Merged

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

merged 2 commits into from Sep 8, 2023

Conversation

Fdawgs
Copy link
Member

@Fdawgs Fdawgs commented Sep 7, 2023

See https://github.com/nodejs/node/pull/37246/files#r588397158 and https://nodejs.org/api/modules.html#core-modules

Checklist

@Fdawgs
Copy link
Member Author

Fdawgs commented Sep 7, 2023

Seem to be getting test failures locally even on master, using both 18.17.1 and 20.6.0

@gurgunday
Copy link
Member

gurgunday commented Sep 7, 2023

It might be because of a test that modifies/mocks a built-in module

@gurgunday
Copy link
Member

Modifying static.test.js (L2822) fixes the issue

t.test(
  'register with rootpath that causes statSync to fail with non-ENOENT code',
  (t) => {
    const fastifyStatic = proxyquire('../', {
      'node:fs': { // HERE: fs -> 'node:fs'
        statSync: function statSyncStub (path) {
          throw new Error({ code: 'MOCK' })
        }
      }
    })

    const pluginOptions = {
      root: path.join(__dirname, '/static'),
      wildcard: true
    }
    const fastify = Fastify()
    fastify.register(fastifyStatic, pluginOptions)

    t.teardown(fastify.close.bind(fastify))
    fastify.listen({ port: 0 }, (err) => {
      fastify.server.unref()
      t.ok(err)
      t.end()
    })
  }
)

Copy link
Member

@Eomm Eomm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was today years old...

Core modules can be identified using the node: prefix, in which case it bypasses the require cache. For instance, require('node:http') will always return the built in HTTP module, even if there is require.cache entry by that name.

Didn't know it

LGTM on green pipeline 👍🏼

@Fdawgs
Copy link
Member Author

Fdawgs commented Sep 8, 2023

Happy to mass update rest of the repos with a similar PR at some point this weekend, unless @gurgunday beats me to it 😂

@Uzlopak
Copy link
Contributor

Uzlopak commented Sep 8, 2023

What about prefixing npm packages with npm:? Wouldnt we then be deno compatible?

@gurgunday
Copy link
Member

gurgunday commented Sep 8, 2023

Happy to mass update rest of the repos with a similar PR at some point this weekend, unless @gurgunday beats me to it 😂

🤣, I just wanted to get the ones I actively use out of the way in case there is a release or something

I would of course prefer if you created a script for it 😁

@Fdawgs
Copy link
Member Author

Fdawgs commented Sep 8, 2023

What about prefixing npm packages with npm:? Wouldnt we then be deno compatible?

Can't find anything in node's documentation about supporting this?

@Fdawgs Fdawgs requested a review from Uzlopak September 8, 2023 12:08
Copy link
Contributor

@Uzlopak Uzlopak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Uzlopak Uzlopak merged commit 4e213cb into fastify:master Sep 8, 2023
15 checks passed
@Fdawgs Fdawgs deleted the perf/builtins branch September 8, 2023 12:50
Fdawgs added a commit to fastify/ajv-compiler that referenced this pull request Sep 10, 2023
Fdawgs added a commit to fastify/aws-lambda-fastify that referenced this pull request Sep 10, 2023
Fdawgs added a commit to fastify/compile-schemas-to-typescript that referenced this pull request Sep 10, 2023
Fdawgs added a commit to fastify/create-fastify that referenced this pull request Sep 10, 2023
Fdawgs added a commit to fastify/csrf-protection that referenced this pull request Sep 10, 2023
Fdawgs added a commit to fastify/deepmerge that referenced this pull request Sep 10, 2023
Fdawgs added a commit to fastify/env-schema that referenced this pull request Sep 10, 2023
Fdawgs added a commit to fastify/fast-content-type-parse that referenced this pull request Sep 10, 2023
Fdawgs added a commit to fastify/fastify that referenced this pull request Sep 10, 2023
Fdawgs added a commit to fastify/fastify-autoload that referenced this pull request Sep 10, 2023
gurgunday pushed a commit to fastify/fastify-bearer-auth that referenced this pull request Sep 10, 2023
gurgunday pushed a commit to fastify/fastify-kafka that referenced this pull request Sep 10, 2023
)

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

See fastify/fastify-static#407

* docs: use `node:` prefix to bypass require.cache call for builtins
gurgunday pushed a commit to fastify/send that referenced this pull request Sep 10, 2023
* perf: use `node:` prefix to bypass require.cache call for builtins

See fastify/fastify-static#407

* docs: use `node:` prefix to bypass require.cache call for builtins
gurgunday pushed a commit to fastify/fastify-early-hints that referenced this pull request Sep 10, 2023
gurgunday pushed a commit to fastify/fastify-multipart that referenced this pull request Sep 10, 2023
)

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

See fastify/fastify-static#407

* docs: use `node:` prefix to bypass require.cache call for builtins
gurgunday pushed a commit to fastify/fastify-oauth2 that referenced this pull request Sep 10, 2023
Uzlopak pushed a commit to fastify/fastify-autoload that referenced this pull request Sep 10, 2023
Uzlopak pushed a commit to fastify/fastify-etag that referenced this pull request Sep 10, 2023
gurgunday pushed a commit to fastify/fastify-env that referenced this pull request Sep 10, 2023
gurgunday pushed a commit to fastify/fastify-http-proxy that referenced this pull request Sep 10, 2023
gurgunday pushed a commit to fastify/fastify-secure-session that referenced this pull request Sep 10, 2023
)

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

See fastify/fastify-static#407

* docs: use `node:` prefix to bypass require.cache call for builtins
Uzlopak pushed a commit to fastify/fast-uri that referenced this pull request Sep 10, 2023
Uzlopak pushed a commit to fastify/fastify-reply-from that referenced this pull request Sep 10, 2023
)

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

See fastify/fastify-static#407

* docs: use `node:` prefix to bypass require.cache call for builtins
Uzlopak added a commit to fastify/pre-commit that referenced this pull request Sep 10, 2023
* perf: use `node:` prefix to bypass require.cache call for builtins

See fastify/fastify-static#407

* try to fix tests

---------

Co-authored-by: Uzlopak <aras.abbasi@googlemail.com>
Uzlopak pushed a commit to fastify/fast-content-type-parse that referenced this pull request Sep 10, 2023
gurgunday pushed a commit to fastify/releasify that referenced this pull request Sep 10, 2023
)

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

See fastify/fastify-static#407

* try to fix

---------

Co-authored-by: Uzlopak <aras.abbasi@googlemail.com>
Uzlopak pushed a commit to fastify/fast-json-stringify that referenced this pull request Sep 10, 2023
)

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

See fastify/fastify-static#407

* docs: use `node:` prefix to bypass require.cache call for builtins
Eomm pushed a commit to fastify/create-fastify that referenced this pull request Sep 11, 2023
mcollina pushed a commit to fastify/proxy-addr that referenced this pull request Sep 12, 2023
* perf: use `node:` prefix to bypass require.cache call for builtins

See fastify/fastify-static#407

* chore: fix linting errors
Fdawgs added a commit to fastify/fastify-cli that referenced this pull request Sep 15, 2023
mcollina pushed a commit to fastify/csrf that referenced this pull request Sep 15, 2023
* perf: use `node:` prefix to bypass require.cache call for builtins

See fastify/fastify-static#407

* chore: fix linting

* test: fix tests
@Fdawgs
Copy link
Member Author

Fdawgs commented Sep 15, 2023

@mcollina happy to update Pino repos accordingly also, wdyt?

renovate bot added a commit to redwoodjs/redwood that referenced this pull request Sep 16, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@fastify/static](https://togithub.com/fastify/fastify-static) |
[`6.11.0` ->
`6.11.1`](https://renovatebot.com/diffs/npm/@fastify%2fstatic/6.11.0/6.11.1)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@fastify%2fstatic/6.11.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@fastify%2fstatic/6.11.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@fastify%2fstatic/6.11.0/6.11.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@fastify%2fstatic/6.11.0/6.11.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>fastify/fastify-static (@&#8203;fastify/static)</summary>

###
[`v6.11.1`](https://togithub.com/fastify/fastify-static/releases/tag/v6.11.1)

[Compare
Source](https://togithub.com/fastify/fastify-static/compare/v6.11.0...v6.11.1)

##### What's Changed

- Remove test/content-type/sample.jpg.br by
[@&#8203;mcollina](https://togithub.com/mcollina) in
[fastify/fastify-static#400
- build(deps-dev): bump tsd from 0.28.1 to 0.29.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/fastify-static#401
- added some small code improvements by
[@&#8203;turnerran](https://togithub.com/turnerran) in
[fastify/fastify-static#399
- (more) small performance/clarity improvements by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[fastify/fastify-static#405
- perf: use `node:` prefix to bypass require.cache call for builtins by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/fastify-static#407
- use native stream by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[fastify/fastify-static#408
- Fix request route config deprecation warnings by
[@&#8203;TTPO100AJIEX](https://togithub.com/TTPO100AJIEX) in
[fastify/fastify-static#409

##### New Contributors

- [@&#8203;turnerran](https://togithub.com/turnerran) made their first
contribution in
[fastify/fastify-static#399
- [@&#8203;TTPO100AJIEX](https://togithub.com/TTPO100AJIEX) made their
first contribution in
[fastify/fastify-static#409

**Full Changelog**:
fastify/fastify-static@v6.11.0...v6.11.1

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/redwoodjs/redwood).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi44My4wIiwidXBkYXRlZEluVmVyIjoiMzYuODMuMCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
jtoar pushed a commit to redwoodjs/redwood that referenced this pull request Sep 16, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@fastify/static](https://togithub.com/fastify/fastify-static) |
[`6.11.0` ->
`6.11.1`](https://renovatebot.com/diffs/npm/@fastify%2fstatic/6.11.0/6.11.1)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@fastify%2fstatic/6.11.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@fastify%2fstatic/6.11.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@fastify%2fstatic/6.11.0/6.11.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@fastify%2fstatic/6.11.0/6.11.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>fastify/fastify-static (@&#8203;fastify/static)</summary>

###
[`v6.11.1`](https://togithub.com/fastify/fastify-static/releases/tag/v6.11.1)

[Compare
Source](https://togithub.com/fastify/fastify-static/compare/v6.11.0...v6.11.1)

##### What's Changed

- Remove test/content-type/sample.jpg.br by
[@&#8203;mcollina](https://togithub.com/mcollina) in
[fastify/fastify-static#400
- build(deps-dev): bump tsd from 0.28.1 to 0.29.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/fastify-static#401
- added some small code improvements by
[@&#8203;turnerran](https://togithub.com/turnerran) in
[fastify/fastify-static#399
- (more) small performance/clarity improvements by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[fastify/fastify-static#405
- perf: use `node:` prefix to bypass require.cache call for builtins by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/fastify-static#407
- use native stream by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[fastify/fastify-static#408
- Fix request route config deprecation warnings by
[@&#8203;TTPO100AJIEX](https://togithub.com/TTPO100AJIEX) in
[fastify/fastify-static#409

##### New Contributors

- [@&#8203;turnerran](https://togithub.com/turnerran) made their first
contribution in
[fastify/fastify-static#399
- [@&#8203;TTPO100AJIEX](https://togithub.com/TTPO100AJIEX) made their
first contribution in
[fastify/fastify-static#409

**Full Changelog**:
fastify/fastify-static@v6.11.0...v6.11.1

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/redwoodjs/redwood).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi44My4wIiwidXBkYXRlZEluVmVyIjoiMzYuODMuMCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Fdawgs added a commit to fastify/fastify-hotwire that referenced this pull request Oct 1, 2023
Fdawgs added a commit to fastify/fastify-cli that referenced this pull request Oct 18, 2023
Fdawgs added a commit to fastify/fastify-cli that referenced this pull request Oct 28, 2023
Eomm pushed a commit to fastify/fastify-cli that referenced this pull request Oct 30, 2023
)

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

See fastify/fastify-static#407

* test(start): fix mock
renovate bot added a commit to specfy/specfy that referenced this pull request Dec 4, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@fastify/static](https://togithub.com/fastify/fastify-static) |
[`6.10.2` ->
`6.12.0`](https://renovatebot.com/diffs/npm/@fastify%2fstatic/6.10.2/6.12.0)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@fastify%2fstatic/6.12.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@fastify%2fstatic/6.12.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@fastify%2fstatic/6.10.2/6.12.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@fastify%2fstatic/6.10.2/6.12.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>fastify/fastify-static (@&#8203;fastify/static)</summary>

###
[`v6.12.0`](https://togithub.com/fastify/fastify-static/releases/tag/v6.12.0)

[Compare
Source](https://togithub.com/fastify/fastify-static/compare/v6.11.2...v6.12.0)

#### What's Changed

- fix eslint by [@&#8203;gurgunday](https://togithub.com/gurgunday) in
[fastify/fastify-static#415
- refactor: add `u` unicode flag to regex by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/fastify-static#418
- refactor: remove unused bindings by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/fastify-static#417
- perf(index): check set size over converting to array to check length
by [@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/fastify-static#416
- chore(package): explicitly declare js module type by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/fastify-static#419
- test: replace `typeof` undefined check by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/fastify-static#420

**Full Changelog**:
fastify/fastify-static@v6.11.2...v6.12.0

###
[`v6.11.2`](https://togithub.com/fastify/fastify-static/releases/tag/v6.11.2)

[Compare
Source](https://togithub.com/fastify/fastify-static/compare/v6.11.1...v6.11.2)

#### What's Changed

- fix: allow fallback to fastify@2.22 route config by
[@&#8203;climba03003](https://togithub.com/climba03003) in
[fastify/fastify-static#410

**Full Changelog**:
fastify/fastify-static@v6.11.1...v6.11.2

###
[`v6.11.1`](https://togithub.com/fastify/fastify-static/releases/tag/v6.11.1)

[Compare
Source](https://togithub.com/fastify/fastify-static/compare/v6.11.0...v6.11.1)

#### What's Changed

- Remove test/content-type/sample.jpg.br by
[@&#8203;mcollina](https://togithub.com/mcollina) in
[fastify/fastify-static#400
- build(deps-dev): bump tsd from 0.28.1 to 0.29.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/fastify-static#401
- added some small code improvements by
[@&#8203;turnerran](https://togithub.com/turnerran) in
[fastify/fastify-static#399
- (more) small performance/clarity improvements by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[fastify/fastify-static#405
- perf: use `node:` prefix to bypass require.cache call for builtins by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[fastify/fastify-static#407
- use native stream by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[fastify/fastify-static#408
- Fix request route config deprecation warnings by
[@&#8203;TTPO100AJIEX](https://togithub.com/TTPO100AJIEX) in
[fastify/fastify-static#409

#### New Contributors

- [@&#8203;turnerran](https://togithub.com/turnerran) made their first
contribution in
[fastify/fastify-static#399
- [@&#8203;TTPO100AJIEX](https://togithub.com/TTPO100AJIEX) made their
first contribution in
[fastify/fastify-static#409

**Full Changelog**:
fastify/fastify-static@v6.11.0...v6.11.1

###
[`v6.11.0`](https://togithub.com/fastify/fastify-static/releases/tag/v6.11.0)

[Compare
Source](https://togithub.com/fastify/fastify-static/compare/v6.10.2...v6.11.0)

#### What's Changed

- Update README with 404+wildcard behavior by
[@&#8203;Ethan-Arrowood](https://togithub.com/Ethan-Arrowood) in
[fastify/fastify-static#390
- add tests for
[#&#8203;234](https://togithub.com/fastify/fastify-static/issues/234) by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[fastify/fastify-static#391
- remove linting from the test script by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[fastify/fastify-static#394
- Remove redundant parameter by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[fastify/fastify-static#395
- build(deps-dev): bump
[@&#8203;typescript-eslint/eslint-plugin](https://togithub.com/typescript-eslint/eslint-plugin)
from 2.34.0 to 6.2.1 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fastify/fastify-static#397
- add url type by [@&#8203;gurgunday](https://togithub.com/gurgunday) in
[fastify/fastify-static#398

#### New Contributors

- [@&#8203;Ethan-Arrowood](https://togithub.com/Ethan-Arrowood) made
their first contribution in
[fastify/fastify-static#390
- [@&#8203;gurgunday](https://togithub.com/gurgunday) made their first
contribution in
[fastify/fastify-static#391

**Full Changelog**:
fastify/fastify-static@v6.10.2...v6.11.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 4pm every weekday" in timezone
Europe/Paris, Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/specfy/specfy).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy41OS44IiwidXBkYXRlZEluVmVyIjoiMzcuODEuMyIsInRhcmdldEJyYW5jaCI6ImNob3JlL3Jlbm92YXRlQmFzZUJyYW5jaCJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

None yet

4 participants