unjs / ipx

๐Ÿ–ผ๏ธ High performance, secure and easy-to-use image optimizer.
MIT License
1.2k stars 59 forks source link

feat(node-fs): add support for multiple dirs #203

Closed Aareksio closed 5 months ago

Aareksio commented 6 months ago

๐Ÿ”— Linked issue

https://github.com/nuxt/image/pull/1177

โ“ Type of change

๐Ÿ“š Description

As discussed, add support for serving files from multiple directories. At the moment of posting this PR the code scans dirs in order and returns first found file, meaning earlier dirs take precedence.

There are no docs in this repository mentioning programatic use of dir and the change is backwards compatible.

๐Ÿ“ Checklist

Aareksio commented 6 months ago

Reading the source, I think it may be better to skip throwing error when file is not found to let the IPX logic handle it with common error code: 1, 2. We currently throw slightly different error message. I find it unnecessary.

codecov[bot] commented 5 months ago

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (be4b092) 53.61% compared to head (daf8729) 56.02%.

Files Patch % Lines
src/storage/node-fs.ts 84.21% 9 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #203 +/- ## ========================================== + Coverage 53.61% 56.02% +2.40% ========================================== Files 14 14 Lines 1188 1203 +15 Branches 66 83 +17 ========================================== + Hits 637 674 +37 + Misses 548 526 -22 Partials 3 3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

pi0 commented 5 months ago

Re error, i mainly used new codes for each driver to make debugging easier against user setups.

pi0 commented 5 months ago

Thanks again! I have pushed few refactors to make it mergable hope you are happy with them and any PR more than welcome to improve โค๏ธ

Aareksio commented 5 months ago

Thank you! I love how you used scoped fs by returning read function. Perhaps I shouldn't have overthinked the forbidden path, you are right this implies invalid configuration :)

I'll push updates to https://github.com/nuxt/image/pull/1177