Export Volume as a class instead of const

[alex996]

I ran into a TS error with the following code:

import { Volume } from 'memfs'

const vol = Volume.fromJSON({}) // works

const getData = (someVol: Volume) => {} // doesn't work
                          ~~~~~~

'Volume' refers to a value, but is being used as a type here. Did you mean 'typeof Volume'?

Using the suggested typeof doesn't help either:

const getData = (someVol: typeof Volume) => {} // works

getData(vol) // doesn't work
        ~~~

Argument of type 'Volume' is not assignable to parameter of type 'typeof Volume'.
  Type 'Volume' is missing the following properties from type 'typeof Volume': prototype, fd

The strange thing is that Volume is already exported as typeof Volume. However, it seems incorrect because in TS

a class declaration creates two things: a type representing instances of the class and a constructor function

as suggested in the Handbook. Indeed, if I import the class directly, the error goes away:

import { Volume } from "memfs/lib/volume";

Any thoughts? Thanks.

[aleung]

I'm facing the same issue. Can this PR be merged?

[corwin-of-amber]

This PR makes sense to me. The use of _Volume seems a bit redundant.

[shorwood]

I agree. As of now, we have to either use: _Volume or InstanceType<Volume>.

This would be a small improvement that respects best-practices and reduces confusion.

[G-Rath]

Closing due to age

[ericmorand]

@G-Rath but the issue still exists. We can't use Volume as a type which makes passing Volume instances between methods more difficult than needed without any reason.

[G-Rath]

@ericmorand feel free to open a new issue with an updated reproduction - I've not saying there isn't a problem, but this PR was a couple of years old and frankly I'm pretty sure this could be a breaking change?

Either way, I'm happy to discuss if someone opens a fresh issue with a modern reproduction (and ideally a TS playground link)

[ericmorand]

@G-Rath, thanks for the answer.

Not really a reproduction, but here is what we have to write to declare a method parameter as a Volume:

import {Volume} from "memfs";

type CreateCompiler = (options: ts.CompilerOptions, volume?: InstanceType<typeof Volume>) => Compiler;

By changing the declaration of Volume in node_modules/memfs/lib/index.d.ts as proposed by this PR, we can use the Volume class as type:

import {Volume} from "memfs";

type CreateCompiler = (options: ts.CompilerOptions, volume?: Volume) => Compiler;

For information, here is the modified declaration file that I used to build the code just above:

import Stats from './Stats';
import Dirent from './Dirent';
import { Volume, StatWatcher, FSWatcher, IWriteStream, DirectoryJSON, NestedDirectoryJSON } from './volume';
import { constants } from './constants';
import type { FsPromisesApi } from './node/types';
import type * as misc from './node/types/misc';
export { Volume, DirectoryJSON, NestedDirectoryJSON };
export declare const vol: Volume;
export interface IFs extends Volume {
    constants: typeof constants;
    Stats: new (...args: any[]) => Stats;
    Dirent: new (...args: any[]) => Dirent;
    StatWatcher: new () => StatWatcher;
    FSWatcher: new () => FSWatcher;
    ReadStream: new (...args: any[]) => misc.IReadStream;
    WriteStream: new (...args: any[]) => IWriteStream;
    promises: FsPromisesApi;
    _toUnixTimestamp: any;
}
export declare function createFsFromVolume(vol: Volume): IFs;
export declare const fs: IFs;
/**
 * Creates a new file system instance.
 *
 * @param json File system structure expressed as a JSON object.
 *        Use `null` for empty directories and empty string for empty files.
 * @param cwd Current working directory. The JSON structure will be created
 *        relative to this path.
 * @returns A `memfs` file system instance, which is a drop-in replacement for
 *          the `fs` module.
 */
export declare const memfs: (json?: NestedDirectoryJSON, cwd?: string) => {
    fs: IFs;
    vol: Volume;
};
export type IFsWithVolume = IFs & {
    __vol: Volume;
};

As you can see, all I did was remove the _Volume alias and export the Volume class untouched. I don't think this would create breaking change, since both my examples compile successfully with the update delcarations, like my TypeScript compiler project that relies heavily on memfs and that uses the InstanceType<typeof Volume>.

[G-Rath]

Either way, I'm happy to discuss if someone opens a fresh issue with a modern reproduction (and ideally a TS playground link)

Also just fair warning, one of my first questions will be "what's the problem with just doing typeof Volume? (aside from the additional typing)"

[ericmorand]

@G-Rath , actually, typeof Volume doesn't work:

The reason is that Volume is already a typeof something - namely a typeof _Volume.

The main issue is that Volume is exported as a value - a const. The nuance with classes is that TypeScript considers classes as both a value and a type, depending on the context. That's why both these are legit when Volume is exported as a class:

import type {Volume} from "memfs"; // this is the type
import {Volume} from "memfs"; // this is the class _and_ the type

I personally always declare interfaces independently of the type of factory that I expose (a class or a classic functional factory). This definitely solves the issue upstream. But exposing a class also works thanks to TypeScript (deliberate) merge between class and type: https://www.typescriptlang.org/docs/handbook/declaration-merging.html#basic-concepts