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

Replacing FileSystem::read_to_string() with typed alternatives #355

Open
arendjr opened this issue Jan 10, 2025 · 2 comments
Open

Replacing FileSystem::read_to_string() with typed alternatives #355

arendjr opened this issue Jan 10, 2025 · 2 comments

Comments

@arendjr
Copy link

arendjr commented Jan 10, 2025

For Biome's multi-file analysis efforts, I want to use oxc-resolver to resolve arbitrary imports from JS files. However, we'll be persisting an in-memory cache of projects as a whole, and I'd like to use this cache to be used for resolving as well.

Great, you say, there's a FileSystem trait, and it can be used to implement in-memory file systems, so why not let the cache implement FileSystem?

Two reasons:

  • Our cache contains parsed information extracted from package.json and the likes. Implementing FileSystem means that we have the supply the contents of such files as strings. Not impossible, but inefficient.
  • Implementing FileSystem::read_to_string() means that we would need to implement support for reading arbitrary files. But upon closer inspection, it seems that it is only used for package.json and tsconfig.json files. This makes quite a difference for how we would implement this functionality: arbitrary file content is stored in a different cache than what we call "project layout" information. It would greatly simplify things for us if we only needed to pull information from the latter.

My suggestion: Replace FileSystem::read_to_string() with two methods FileSystem::read_package_json() and FileSystem::read_tsconfig_json().

I realize these requirements are very Biome-specific, so I understand if we may need to fork and go our own way. But I wanted to ask if there's any interest in following the same approach as Biome for oxc-resolver, in which case I'll be happy to contribute back our changes. Happy to hear your thoughts!

@Boshen
Copy link
Member

Boshen commented Jan 10, 2025

Would you like to make the changes with Biome integrated? I'm happy to integrate your changes back to oxlint and rolldown. Perhaps this will also help us work towards #236.

@arendjr
Copy link
Author

arendjr commented Jan 10, 2025

Yeah, sounds like the requirements for #236 and this are quite similar indeed. I'm indeed first trying it out in Biome to validate the changes. If that works out I'll create a PR 👍

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

No branches or pull requests

2 participants