Skip to content

Conversation

@ilteoood
Copy link
Contributor

@ilteoood ilteoood commented Oct 9, 2025

PR to introduce small improvements, like reading the url.length just once and avoid invoking a function to retrieve the encoding extension.

Checklist

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.

I did not test my suggestions...

@Fdawgs Fdawgs changed the title feat: small improvements refactor: small improvements Oct 10, 2025
ilteoood and others added 2 commits October 13, 2025 23:06
Co-authored-by: Aras Abbasi <[email protected]>
Signed-off-by: Matteo Pietro Dazzi <[email protected]>
@ilteoood ilteoood requested a review from Uzlopak October 13, 2025 21:13
@ilteoood
Copy link
Contributor Author

I had to remove the || '' fallback, otherwise a test was failing

@Uzlopak
Copy link
Contributor

Uzlopak commented Oct 13, 2025

which test would have failed when doing || '' ?

@ilteoood
Copy link
Contributor Author

ilteoood commented Oct 13, 2025

which test would have failed when doing || '' ?
Here is the log:

✖ failing tests:

test at test/static.test.js:2684:1
✖ will serve uncompressed files if there are no compressed variants on disk (2.138959ms)
 AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
 + actual - expected
 
 + 'deflate'
 - undefined
     at assert.<computed> [as deepStrictEqual] (node:internal/test_runner/test:242:18)
     at TestContext.<anonymous> (/Users/ilteoood/Documents/git/fastify/fastify-static/test/static.test.js:2707:14)
     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
     at async Test.run (node:internal/test_runner/test:797:9)
     at async Test.processPendingSubtests (node:internal/test_runner/test:526:7) {
   generatedMessage: true,
   code: 'ERR_ASSERTION',
   actual: 'deflate',
   expected: undefined,
   operator: 'deepStrictEqual'
 }

test at test/static.test.js:2833:1
✖ will serve uncompressed files if there are no compressed variants on disk (with wildcard: false) (2.539583ms)
 AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
 + actual - expected
 
 + 'deflate'
 - undefined
     at assert.<computed> [as deepStrictEqual] (node:internal/test_runner/test:242:18)
     at TestContext.<anonymous> (/Users/ilteoood/Documents/git/fastify/fastify-static/test/static.test.js:2857:14)
     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
     at async Test.run (node:internal/test_runner/test:797:9)
     at async Test.processPendingSubtests (node:internal/test_runner/test:526:7) {
   generatedMessage: true,
   code: 'ERR_ASSERTION',
   actual: 'deflate',
   expected: undefined,
   operator: 'deepStrictEqual'
 }

@ilteoood ilteoood requested a review from Uzlopak October 13, 2025 21:49
@Fdawgs Fdawgs requested a review from Copilot October 14, 2025 19:20
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces small performance and code quality improvements to the fastify-static plugin, including optimizing property access patterns and replacing function calls with direct object lookups.

  • Replaced multiple conditional assignments with nullish coalescing operators for cleaner code
  • Optimized encoding extension lookup by replacing function calls with direct object mapping
  • Improved loop performance by caching url.length in a variable

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@ilteoood ilteoood merged commit ac71e33 into main Oct 16, 2025
17 checks passed
@Uzlopak Uzlopak deleted the feat/small-improvements branch October 16, 2025 07:42
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.

5 participants