Skip to content

added some small code improvements#399

Merged
Uzlopak merged 10 commits into
fastify:masterfrom
turnerran:some-code-improvments
Sep 6, 2023
Merged

added some small code improvements#399
Uzlopak merged 10 commits into
fastify:masterfrom
turnerran:some-code-improvments

Conversation

@turnerran

@turnerran turnerran commented Sep 2, 2023

Copy link
Copy Markdown
Contributor

Checklist

@turnerran turnerran changed the title added some code improvements added some small code improvements Sep 2, 2023
jsumners
jsumners reviewed Sep 2, 2023
View reviewed changes
Comment thread lib/dirList.js Outdated
@turnerran turnerran requested a review from jsumners September 2, 2023 20:59
jsumners
jsumners reviewed Sep 2, 2023
View reviewed changes
Comment thread lib/dirList.js Outdated
Fdawgs
Fdawgs requested changes Sep 3, 2023
View reviewed changes

@Fdawgs Fdawgs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the optional chaining changes are good, but the rest are just stylistic and don't really improve on anything.

@turnerran turnerran requested review from Fdawgs and jsumners September 3, 2023 14:05
gurgunday
gurgunday reviewed Sep 3, 2023
View reviewed changes
Comment thread lib/dirList.js Outdated
@turnerran @gurgunday
Updated second condition
1dfcfdd 
Co-authored-by: Gürgün Dayıoğlu <gurgun.dayioglu@icloud.com>
@turnerran turnerran requested a review from gurgunday September 3, 2023 15:54
gurgunday
gurgunday reviewed Sep 3, 2023
View reviewed changes
Comment thread lib/dirList.js
@turnerran @gurgunday
Updated return value
e701c97 
Co-authored-by: Gürgün Dayıoğlu <gurgun.dayioglu@icloud.com>
@mcollina

mcollina commented Sep 4, 2023

Copy link
Copy Markdown
Member

CI seems to be failing.

@mcollina

mcollina commented Sep 4, 2023

Copy link
Copy Markdown
Member

Please rebase on top of master.

@turnerran

turnerran commented Sep 4, 2023

Copy link
Copy Markdown
Contributor Author

Please rebase on top of master.

Hey @mcollina , I'm up to date with master

@mcollina

mcollina commented Sep 5, 2023

Copy link
Copy Markdown
Member

I've updated it for you.

Uzlopak
Uzlopak reviewed Sep 5, 2023
View reviewed changes
Comment thread lib/dirList.js Outdated
@turnerran @Uzlopak
Updated condition to return boolean
e2ba99a 
Co-authored-by: Uzlopak <aras.abbasi@googlemail.com>
@turnerran turnerran requested a review from Uzlopak September 5, 2023 19:40
Uzlopak
Uzlopak reviewed Sep 6, 2023
View reviewed changes
Comment thread lib/dirList.js Outdated
@turnerran @Uzlopak
Removed redundant return type
6108eae 
Co-authored-by: Uzlopak <aras.abbasi@googlemail.com>
Uzlopak
Uzlopak approved these changes Sep 6, 2023
View reviewed changes

@Uzlopak Uzlopak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@Uzlopak

Uzlopak commented Sep 6, 2023

Copy link
Copy Markdown
Contributor

@Fdawgs PTAL

gurgunday
gurgunday reviewed Sep 6, 2023
View reviewed changes
Comment thread lib/dirList.js Outdated
@turnerran @gurgunday
Added nullish operator for performance optimization
59da0f0 
Co-authored-by: Gürgün Dayıoğlu <gurgun.dayioglu@icloud.com>
@turnerran turnerran requested a review from gurgunday September 6, 2023 12:54
gurgunday
gurgunday approved these changes Sep 6, 2023
View reviewed changes

@gurgunday gurgunday left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@Uzlopak

Uzlopak commented Sep 6, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Btw. I am old school and never benchmarked the benefit of || vs. ??. Keep in mind, that || can be used in branchless programming. Personally I never checked how ?? is implemented in nodejs. So should be in cases like calling another function, ?? func() definetly be slower, but in cases where we do || false vs. ?? false that || false be faster . But I am fine with this implementation.

@Uzlopak Uzlopak merged commit 4499e75 into fastify:master Sep 6, 2023
@gurgunday

gurgunday commented Sep 6, 2023

Copy link
Copy Markdown
Member

Btw. I am old school and never benchmarked the benefit of || vs. ??. Keep in mind, that || can be used in branchless programming. Personally I never checked how ?? is implemented in nodejs. So should be in cases like calling another function, ?? func() definetly be slower, but in cases where we do || false vs. ?? false that || false be faster . But I am fine with this implementation.

I see, I thought it'd be a clear case since ?? checks for less things (2 falsy values instead of 6) and short circuits more often – I'll keep this in mind

renovate Bot referenced this pull request in redwoodjs/graphql Sep 16, 2023
@renovate
fix(deps): update dependency @fastify/static to v6.11.1 (#9182)
552eec3 
[![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
[https://github.com/fastify/fastify-static/pull/400](https://togithub.com/fastify/fastify-static/pull/400)
- build(deps-dev): bump tsd from 0.28.1 to 0.29.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/fastify/fastify-static/pull/401](https://togithub.com/fastify/fastify-static/pull/401)
- added some small code improvements by
[@&#8203;turnerran](https://togithub.com/turnerran) in
[https://github.com/fastify/fastify-static/pull/399](https://togithub.com/fastify/fastify-static/pull/399)
- (more) small performance/clarity improvements by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[https://github.com/fastify/fastify-static/pull/405](https://togithub.com/fastify/fastify-static/pull/405)
- perf: use `node:` prefix to bypass require.cache call for builtins by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[https://github.com/fastify/fastify-static/pull/407](https://togithub.com/fastify/fastify-static/pull/407)
- use native stream by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[https://github.com/fastify/fastify-static/pull/408](https://togithub.com/fastify/fastify-static/pull/408)
- Fix request route config deprecation warnings by
[@&#8203;TTPO100AJIEX](https://togithub.com/TTPO100AJIEX) in
[https://github.com/fastify/fastify-static/pull/409](https://togithub.com/fastify/fastify-static/pull/409)

##### New Contributors

- [@&#8203;turnerran](https://togithub.com/turnerran) made their first
contribution in
[https://github.com/fastify/fastify-static/pull/399](https://togithub.com/fastify/fastify-static/pull/399)
- [@&#8203;TTPO100AJIEX](https://togithub.com/TTPO100AJIEX) made their
first contribution in
[https://github.com/fastify/fastify-static/pull/409](https://togithub.com/fastify/fastify-static/pull/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 referenced this pull request in redwoodjs/graphql Sep 16, 2023
@renovate @jtoar
fix(deps): update dependency @fastify/static to v6.11.1 (#9182)
68f8ddc 
[![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
[https://github.com/fastify/fastify-static/pull/400](https://togithub.com/fastify/fastify-static/pull/400)
- build(deps-dev): bump tsd from 0.28.1 to 0.29.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/fastify/fastify-static/pull/401](https://togithub.com/fastify/fastify-static/pull/401)
- added some small code improvements by
[@&#8203;turnerran](https://togithub.com/turnerran) in
[https://github.com/fastify/fastify-static/pull/399](https://togithub.com/fastify/fastify-static/pull/399)
- (more) small performance/clarity improvements by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[https://github.com/fastify/fastify-static/pull/405](https://togithub.com/fastify/fastify-static/pull/405)
- perf: use `node:` prefix to bypass require.cache call for builtins by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[https://github.com/fastify/fastify-static/pull/407](https://togithub.com/fastify/fastify-static/pull/407)
- use native stream by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[https://github.com/fastify/fastify-static/pull/408](https://togithub.com/fastify/fastify-static/pull/408)
- Fix request route config deprecation warnings by
[@&#8203;TTPO100AJIEX](https://togithub.com/TTPO100AJIEX) in
[https://github.com/fastify/fastify-static/pull/409](https://togithub.com/fastify/fastify-static/pull/409)

##### New Contributors

- [@&#8203;turnerran](https://togithub.com/turnerran) made their first
contribution in
[https://github.com/fastify/fastify-static/pull/399](https://togithub.com/fastify/fastify-static/pull/399)
- [@&#8203;TTPO100AJIEX](https://togithub.com/TTPO100AJIEX) made their
first contribution in
[https://github.com/fastify/fastify-static/pull/409](https://togithub.com/fastify/fastify-static/pull/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>
renovate Bot referenced this pull request in specfy/specfy Dec 4, 2023
@renovate
fix(deps): update dependency @fastify/static to v6.12.0 (#393)
dbb8a5c 
[![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
[https://github.com/fastify/fastify-static/pull/415](https://togithub.com/fastify/fastify-static/pull/415)
- refactor: add `u` unicode flag to regex by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[https://github.com/fastify/fastify-static/pull/418](https://togithub.com/fastify/fastify-static/pull/418)
- refactor: remove unused bindings by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[https://github.com/fastify/fastify-static/pull/417](https://togithub.com/fastify/fastify-static/pull/417)
- perf(index): check set size over converting to array to check length
by [@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[https://github.com/fastify/fastify-static/pull/416](https://togithub.com/fastify/fastify-static/pull/416)
- chore(package): explicitly declare js module type by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[https://github.com/fastify/fastify-static/pull/419](https://togithub.com/fastify/fastify-static/pull/419)
- test: replace `typeof` undefined check by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[https://github.com/fastify/fastify-static/pull/420](https://togithub.com/fastify/fastify-static/pull/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
[https://github.com/fastify/fastify-static/pull/410](https://togithub.com/fastify/fastify-static/pull/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
[https://github.com/fastify/fastify-static/pull/400](https://togithub.com/fastify/fastify-static/pull/400)
- build(deps-dev): bump tsd from 0.28.1 to 0.29.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/fastify/fastify-static/pull/401](https://togithub.com/fastify/fastify-static/pull/401)
- added some small code improvements by
[@&#8203;turnerran](https://togithub.com/turnerran) in
[https://github.com/fastify/fastify-static/pull/399](https://togithub.com/fastify/fastify-static/pull/399)
- (more) small performance/clarity improvements by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[https://github.com/fastify/fastify-static/pull/405](https://togithub.com/fastify/fastify-static/pull/405)
- perf: use `node:` prefix to bypass require.cache call for builtins by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[https://github.com/fastify/fastify-static/pull/407](https://togithub.com/fastify/fastify-static/pull/407)
- use native stream by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[https://github.com/fastify/fastify-static/pull/408](https://togithub.com/fastify/fastify-static/pull/408)
- Fix request route config deprecation warnings by
[@&#8203;TTPO100AJIEX](https://togithub.com/TTPO100AJIEX) in
[https://github.com/fastify/fastify-static/pull/409](https://togithub.com/fastify/fastify-static/pull/409)

#### New Contributors

- [@&#8203;turnerran](https://togithub.com/turnerran) made their first
contribution in
[https://github.com/fastify/fastify-static/pull/399](https://togithub.com/fastify/fastify-static/pull/399)
- [@&#8203;TTPO100AJIEX](https://togithub.com/TTPO100AJIEX) made their
first contribution in
[https://github.com/fastify/fastify-static/pull/409](https://togithub.com/fastify/fastify-static/pull/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
[https://github.com/fastify/fastify-static/pull/390](https://togithub.com/fastify/fastify-static/pull/390)
- add tests for
[#&#8203;234](https://togithub.com/fastify/fastify-static/issues/234) by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[https://github.com/fastify/fastify-static/pull/391](https://togithub.com/fastify/fastify-static/pull/391)
- remove linting from the test script by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[https://github.com/fastify/fastify-static/pull/394](https://togithub.com/fastify/fastify-static/pull/394)
- Remove redundant parameter by
[@&#8203;gurgunday](https://togithub.com/gurgunday) in
[https://github.com/fastify/fastify-static/pull/395](https://togithub.com/fastify/fastify-static/pull/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
[https://github.com/fastify/fastify-static/pull/397](https://togithub.com/fastify/fastify-static/pull/397)
- add url type by [@&#8203;gurgunday](https://togithub.com/gurgunday) in
[https://github.com/fastify/fastify-static/pull/398](https://togithub.com/fastify/fastify-static/pull/398)

#### New Contributors

- [@&#8203;Ethan-Arrowood](https://togithub.com/Ethan-Arrowood) made
their first contribution in
[https://github.com/fastify/fastify-static/pull/390](https://togithub.com/fastify/fastify-static/pull/390)
- [@&#8203;gurgunday](https://togithub.com/gurgunday) made their first
contribution in
[https://github.com/fastify/fastify-static/pull/391](https://togithub.com/fastify/fastify-static/pull/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

Reviewers

@Uzlopak Uzlopak Uzlopak approved these changes

@Fdawgs Fdawgs Fdawgs approved these changes

@gurgunday gurgunday gurgunday approved these changes

@jsumners jsumners Awaiting requested review from jsumners

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@turnerran @mcollina @Uzlopak @gurgunday @jsumners @Fdawgs