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

[Suggestion] Expose Volume as a class instead of a const #976

Open
ericmorand opened this issue Dec 21, 2023 · 0 comments
Open

[Suggestion] Expose Volume as a class instead of a const #976

ericmorand opened this issue Dec 21, 2023 · 0 comments

Comments

@ericmorand
Copy link

ericmorand commented Dec 21, 2023

This is a follow-up to the conversation that happened on #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.

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

1 participant