Skip to content

refactor(core): remove FsAdapter.readFile()#2933

Closed
iuioiua wants to merge 1 commit into
freshframework:mainfrom
iuioiua:remove-FsAdapter-readFile
Closed

refactor(core): remove FsAdapter.readFile()#2933
iuioiua wants to merge 1 commit into
freshframework:mainfrom
iuioiua:remove-FsAdapter-readFile

Conversation

@iuioiua

@iuioiua iuioiua commented May 13, 2025

Copy link
Copy Markdown
Contributor

I realise we complicate a lot of objects, classes and interfaces for testing purposes. By stubbing (and possibly) spying only where we need to, we can simplify these symbols and jump through far fewer mental hoops to reason about things.

Pre-requisite for #2934

@iuioiua iuioiua marked this pull request as ready for review May 13, 2025 06:49
@csvn

csvn commented May 13, 2025

Copy link
Copy Markdown
Contributor

I think perhaps FsAdapter exists so that it's possible for Fresh to eventually run on e.g. NodeJS, Cloudflare or other non-Deno environments.

@iuioiua

iuioiua commented May 13, 2025

Copy link
Copy Markdown
Contributor Author

I think FsAdapter exists more for having testing and implementation share the same interface. IMO it's best not to complicate implementation for the sake of testing. You can see this in other spots of the codebase which means the reader has to jump through hoops to figure out what's going on. While it's possible it might be for runtime compatibility, I'm not aware of any such plans.

@iuioiua

iuioiua commented May 14, 2025

Copy link
Copy Markdown
Contributor Author

Closing in favour of another upcoming PR.

@iuioiua iuioiua closed this May 14, 2025
@iuioiua iuioiua deleted the remove-FsAdapter-readFile branch May 14, 2025 07:40
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