memfs icon indicating copy to clipboard operation
memfs copied to clipboard

[Suggestion] Expose `Volume` as a class instead of a `const`

Open ericmorand opened this issue 2 years ago • 0 comments

This is a follow-up to the conversation that happened on https://github.com/streamich/memfs/pull/585.

To summarize: as of today, memfs exports the Volume class as a const value, instead of a class. This makes using the type less elegant and more complicated than needed:

import {Volume} from "memfs";

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

In this example, I have to use the non-obvious InstanceType<typeof Volume> type to make sure that the compiler accepts a Volume instance as argument for the method. Using the more obvious Volume type is not accepted by the compiler because it is a value:

import {Volume} from "memfs";

type CreateCompiler = (
    options: ts.CompilerOptions,
    volume?: Volume
) => Compiler;
$ tsc src/lib/compiler.ts 
src/lib/compiler.ts:60:14 - error TS2749: 'Volume' refers to a value, but is being used as a type here. Did you mean 'typeof Volume'?

60     volume?: Volume
                ~~~~~~

If Volume was exported as a class, it would become possible to use it as a type as well as a value (the class itself), as per TypeScript declaration merging rule: https://www.typescriptlang.org/docs/handbook/declaration-merging.html#basic-concepts

Note that my testing with the declaration file emitted by memfs did demonstrate that exporting Volume as a class doesn't create any breaking change:

  • InstanceType<typeof Volume> continues to work - which means that people that are already using this syntax would not be impacted
  • No memfs method used by my compiler showcased any issue

For reference, the modified declaration file that I've used to conduct my test is this one:

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 { DirectoryJSON, NestedDirectoryJSON, Volume };
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;
};

The only changes are the removal of the _Volume alias, the re-export of Volume and the refactoring due to the removal of _Volume. Basically, exactly what the mentionned PR does.

ericmorand avatar Dec 21 '23 22:12 ericmorand