Add virtual files and refactor snap fetching

[ritave]

I require file metadata be attached to them for #847.

This PR adds:

• A virtual file representation of file contents. It includes things like original path, canonical path, and can be extended by other metadata along by other modules.
  • In the future, it could also be used for fixing the manifest in the cli as well. This is generally done by attaching messages to vfile during the linting.
• I've refactored snap fetching. All fetching is now external to SnapController and is injected into it during initalisation.
• All tests now don't mock internal fetchSnaps function, and inject a mock instead.

closes #1005

[codecov-commenter]

Codecov Report

Merging #1019 (e583607) into main (6162daf) will increase coverage by 0.30%.
The diff coverage is 99.58%.

@@            Coverage Diff             @@
##             main    #1019      +/-   ##
==========================================
+ Coverage   93.02%   93.33%   +0.30%     
==========================================
  Files          96       95       -1     
  Lines        9690     9914     +224     
  Branches      909      935      +26     
==========================================
+ Hits         9014     9253     +239     
+ Misses        676      661      -15     

Impacted Files	Coverage Δ
packages/snaps-utils/src/snaps.ts	98.76% <98.55%> (-0.08%)	⬇️
packages/snaps-utils/src/types.ts	100.00% <100.00%> (ø)
...ckages/snaps-utils/src/virtual-file/VirtualFile.ts	100.00% <100.00%> (ø)
...ages/snaps-utils/src/virtual-file/toVirtualFile.ts	100.00% <100.00%> (ø)
packages/snaps-utils/src/manifest/index.browser.ts
packages/snaps-utils/src/manifest/index.ts

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[kenhkan]

Hey @ritave! I want to dabble into reviewing this but don't have too much time to really meaningfully dig deep into this. Can you enlighten me on a few questions that I have?

1. Can you document why the vfile is the approach that you picked and what are the advantages of using this approach over others? I'm indifferent to whatever the solution we end up using, though I would recommend leaving a trail on the why behind major architectural/structural changes.
2. This does not solve Snap checksum should include all inputs in the calculation #847 and only lays the groundwork for the solution for it, correct? Is there a reason behind separating the actual solution from this PR? It would show how and why vfile would be the preferred approach for the problem that we are solving at hand.

[ritave]

Hell @kenhkan! I'm happy you want to help out with reviewing code!

1)

I suggest you start by reading code comments - a lot of time has been spent making sure all non-obvious architecture decisions are explained inside the code itself right near the location in which the unusual decision has been taken, along with extensive TSDoc documentation for all the exposed APIs which we write for any changes. If you find anything confusing after that, you'll find further multiple and rather exhaustive elaborations in review comments.

While reading code you'll probably notice that the main architecture change is a simple Dependency Injection along with Factory pattern that are taught in the first/second year of university and should be instantly recognisable to any developer.

If you have any further questions after reading all of the explanations already written, please do post them - I'm happy to elaborate since that'll help me understand which things that might seem obvious to me, might not be obvious to you, so that I can learn what should I focus on next time.

2)

As you probably know, due your to experience of being an engineer, smaller PRs are started being reviewed earlier, more enjoyable to the reviewers, reviewed quicker, merged sooner and receive more useful scrutiny. And this user story has outgrown the scope of what I would consider a sensibly-sized PR.

Additionally having separate PRs allows us to discuss decoupled technical decisions on their own merits, and influence further work done.

Adnotations

I'm happy to provide even more documentation to non-engineer stakeholders so that they can verify, measure and understand the work being done, though since MetaMask engineers conduct work publicly in an open-source projects with a _very_ high visibility to the public, may I suggest such documentation be discussed in other venues rather than public repositories, which are geared towards external people, specifically engineers reading the code.

MetaMask is built by people who are strongly driven by the mission and runs on trust, trust extended to our colleagues, trust given to us by the community, and finally, trust to do the right thing out of own volition.

[Mrtenz]

Looks good overall. Just some minor comments.

[Mrtenz]

Why do we need this assertion?

[ritave]

This was copied from previous npm fetching tests.

Fetch mock allows returning streams instead of simple strings

[ritave]

We've internally decided to leave declare way of adding properties to VirtualFile.data, due to benefit/time invested ratio of that refactor being low.

In the future, if time allows, we could research moving to generics instead.