Review requested:

• @nodejs/single-executable
• @nodejs/test_runner

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @avivkeller.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

Nice! This is a great addition. Since it's such a large PR, this will take me some time to review. Will try to tackle it over the next week.

Left an initial review, but like @Ethan-Arrowood said, it'll take time for a more in depth look

It's nice to see some momentum in this area, though from a first glance it seems the design has largely overlooked the feedback from real world use cases collected 4 years ago: https://github.com/nodejs/single-executable/blob/main/docs/virtual-file-system-requirements.md - I think it's worth checking that the API satisfies the constraints that users of this feature have provided, to not waste the work that have been done by prior contributors to gather them, or having to reinvent it later (possibly in a breaking manner) to satisfy these requirements from real world use cases.

Codecov Report

❌ Patch coverage is 92.99967% with 635 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.82%. Comparing base (e0928d6) to head (b047092).
⚠️ Report is 26 commits behind head on main.

@@            Coverage Diff             @@
##             main   #61478      +/-   ##
==========================================
+ Coverage   89.68%   89.82%   +0.13%     
==========================================
  Files         676      692      +16     
  Lines      206555   215687    +9132     
  Branches    39552    41259    +1707     
==========================================
+ Hits       185249   193739    +8490     
- Misses      13444    14061     +617     
- Partials     7862     7887      +25     

And why not something like OPFS aka whatwg/fs?

const rootHandle = await navigator.storage.getDirectory()
await rootHandle.getFileHandle('config.json', { create: true })
fs.mount('/app', rootHandle) // to make it work with fs
fs.readFileSync('/app/config.json')

OR

const rootHandle = await navigator.storage.getDirectory()
await rootHandle.getFileHandle('config.json', { create: true })

fs.readFileSync('sandbox:/config.json')

fs.createVirtual seems like something like a competing specification

And why not something like OPFS aka whatwg/fs?

I generally prefer not to interleave with WHATWG specs as much as possible for core functionality (e.g., SEA). In my experience, they tend to perform poorly on our codebase and remove a few degrees of flexibility. (I also don't find much fun in working on them, and I'm way less interested in contributing to that.)

On an implementation side, the core functionality of this feature will be identical (technically, it's missing writes that OPFS supports), as we would need to impact all our internal fs methods anyway.

If this lands, we can certainly iterate on a WHATWG-compatible API for this, but I would not add this to this PR.

Small prior art: https://github.com/juliangruber/subfs

I also worked on this a bit on the side recently: Qard@73b8fc6

That is very much in chaotic ideation stage with a bunch of LLM assistance to try some different ideas, but the broader concept I was aiming for was to have a VirtualFileSystem type which would actually implement the entire API surface of the fs module, accepting a Provider type to delegate the internals of the whole cluster of file system types to a singular class managing the entire cluster of fs-related types such that the fs module could actually just be fully converted to:

module.exports = new VirtualFileSystem(new LocalProvider())

I intended for it to be extensible for a bunch of different interesting scenarios, so there's also an S3 provider and a zip file provider there, mainly just to validate that the model can be applied to other varieties of storage systems effectively.

Keep in mind, like I said, the current state is very much just ideation in a branch I pushed up just now to share, but I think there are concepts for extensibility in there that we could consider to enable a whole ecosystem of flexible storage providers. 🙂

Personally, I would hope for something which could provide both read and write access through an abstraction with swappable backends of some variety, this way we could pass around these virtualized file systems like objects and let an ecosystem grow around accepting any generalized virtual file system for its storage backing. I think it'd be very nice for a lot of use cases like file uploads or archive management to be able to just treat them like any other readable and writable file system.

Personally, I would hope for something which could provide both read and write access through an abstraction with swappable backends of some variety, this way we could pass around these virtualized file systems like objects and let an ecosystem grow around accepting any generalized virtual file system for its storage backing. I think it'd be very nice for a lot of use cases like file uploads or archive management to be able to just treat them like any other readable and writable file system.

just a bit off topic... but this reminds me of why i created this feature request:
Blob.from() for creating virtual Blobs with custom backing storage

Would not lie, it would be cool if NodeJS also provided some type of static Blob.from function to create virtual lazy blobs. could live on fs.blobFrom for now...

example that would only work in NodeJS (based on how it works internally)

const size = 26

const blobPart = BlobFrom({
  size,
  stream (start, end) {
    // can either be sync or async (that resolves to a ReadableStream)
    // return new Response('abcdefghijklmnopqrstuvwxyz'.slice(start, end)).body
    // return new Blob(['abcdefghijklmnopqrstuvwxyz'.slice(start, end)]).stream()
    
    return fetch('https://httpbin.dev/range/' + size, {
      headers: {
        range: `bytes=${start}-${end - 1}`
      }
    }).then(r => r.body)
  }
})

blobPart.text().then(text => {
  console.log('a-z', text)
})

blobPart.slice(-3).text().then(text => {
  console.log('x-z', text)
})

const a = blobPart.slice(0, 6)
a.text().then(text => {
  console.log('a-f', text)
})

const b = a.slice(2, 4)
b.text().then(text => {
  console.log('c-d', text)
})

x-z xyz
a-z abcdefghijklmnopqrstuvwxyz
a-f abcdef
c-d cd

An actual working PoC

(I would not rely on this unless it became officially supported by nodejs core - this is a hack)

const blob = new Blob()
const symbols = Object.getOwnPropertySymbols(blob)
const blobSymbol = symbols.map(s => [s.description, s])
const symbolMap = Object.fromEntries(blobSymbol)
const {
  kHandle,
  kLength,
} = symbolMap

function BlobFrom ({ size, stream }) {
  const blob = new Blob()
  if (size === 0) return blob

  blob[kLength] = size
  blob[kHandle] = {
    span: [0, size],

    getReader () {
      const [start, end] = this.span
      if (start === end) {
        return { pull: cb => cb(0) }
      }

      let reader

      return {
        async pull (cb) {
          reader ??= (await stream(start, end)).getReader()
          const {done, value} = await reader.read()
          cb(done ^ 1, value)
        }
      }
    },

    slice (start, end) {
      const [baseStart] = this.span

      return {
        span: [baseStart + start, baseStart + end],
        getReader: this.getReader,
        slice: this.slice,
      }
    }
  }

  return blob
}

currently problematic to do: new Blob([a, b]), new File([blobPart], 'alphabet.txt', { type: 'text/plain' })

also need to handle properly clone, serialize & deserialize, if this where to be sent of to another worker - then i would transfer a MessageChannel where the worker thread asks main frame to hand back a transferable ReadableStream when it needs to read something.

but there are probably better ways to handle this internally in core with piping data directly to and from different destinations without having to touch the js runtime? - if only getReader could return the reader directly instead of needing to read from the ReadableStream using js?

Can folks chiming in on the meta discussions about AI/LLM move to #62105 or a separate thread about this topic specifically? GitHub doesn’t handle long threads very well. This PR is already quite difficult to track for technical reviews, please keep the discussions here specific to this PR (question and answers about the copyright analysis are okay as they are technically specific to the content of this PR; more meta discussions about tools can use a dedicated space elsewhere or they will quickly be buried in GitHub’s thread folding along with technical reviews).

I've marked my analysis comment as resolved as I believe it made the point I was trying to make. For the remaining technical discussions, I really think it's going to be easier to do incremental follow-ups than it will be to continue refining this beast of a PR. But, agreed... The rest of the discussion should be focused on the technical side of things

are there any plans to support command execution inside the vfs? like

const vfs = require('node:vfs');
const fs = require('node:fs');

// Create a VFS with default MemoryProvider
const myVfs = vfs.create();

myVfs.exec('ls /')
myVfs.spwan('npm i')

I'm locking this PR on the grounds that it is becoming off-topic and diverging from technical discussions. This is an impartial lock to ensure this PR is on-track.

Questions and concerns regarding AI-practices directly related to this PR or not, please follow to the linked issue on OpenJS Foundation

On a separate note, and not connected to the lock, I fully agree with @jasnell on all his grounds and do believe that the elusive assertion of the DCO not being fullfiled should be prompted elsewhere instead of directly on this PR. OpenJS Foundation Legal cleared the DCO is correctly being followed, so any further deliberation on the matter won't be useful for the development of this PR.

Thank you!

Should this live behind a CLI flag initially? It might be better to work through #62328 while the API is not exposed by default.

Should this live behind a CLI flag initially?

Honestly... wouldn't hurt.

Should this live behind a CLI flag initially? It might be better to work through #62328 while the API is not exposed by default.

The biggest risk of this PR is the patches on the modules' loading side and the fs side. You cannot put them behind a flag. The rest is just a new API with bugs (like many others).

Could those patches land separately from the flaggable part? That way it’d be much easier to ensure no regressions from either portion.

(i ofc wouldn’t suggest splitting the PR until after it was otherwise philosophically landable)

@ljharb maybe. But we would be landing code that is not tested, so we’d be in a worse stance?

Note that experimental features are covered by our threat model.

Surely either the patches are meant to be noops in isolation (in which case existing tests should be sufficient - or improved - to assure no breakage) or have independent functionality (which could ship with its own tests)?

I don’t understand why. Ease of review?

Ease of review, and if there’s any risk in patching core subsystems, that PR could also be released as a patch and then that assumption would be tested, and the VFS functionality could land separately as a minor.

Certainly not required, but it might be prudent for assuring stability.

I’m moving this back to draft. I’m doing some refactoring and simplifying some paths for ease of review. I’ll add a review guide in the PR description and an architecture diagram explaining how all pieces fit together.