feat: define core types for virtual file system by codewizdave · Pull Request #12 · nesalia-inc/vfs

codewizdave · 2026-03-05T10:45:10Z

Summary

Add core TypeScript interfaces for the virtual file system: FileSystemNode, File, Directory, and FileSystem
Add type guards isFile() and isDirectory() for runtime type checking
Add unit tests for type guards

Test plan

Run pnpm test - all tests pass
Run pnpm typecheck - no type errors

🤖 Generated with Claude Code

- Add FileSystemNode, File, Directory interfaces - Add FileSystem interface with CRUD operations - Add isFile and isDirectory type guards - Add tests for type guards Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

marty-action · 2026-03-05T10:46:38Z

+  root: Directory;
+  exists(path: string): boolean;
+  get(path: string): FileSystemItem | null;
+  mkdir(path: string): Directory;


Consider adding return type documentation or handling for existing paths:

mkdir: Should it throw if the directory already exists, or return the existing one?

touch: Should it update content if the file exists, or throw?

marty-action · 2026-03-05T10:46:43Z

+import { isFile, isDirectory, File, Directory } from "./types";
+
+describe("types", () => {
+  describe("isFile", () => {


Consider adding tests for edge cases like null/undefined input to ensure type guards are robust.

marty-action · 2026-03-05T10:46:51Z

+
+export type FileSystemItem = File | Directory;
+
+export interface FileSystem {


The FileSystem interface doesn't specify error handling behavior. Consider documenting:

What happens when paths don't exist (return null vs throw)

Path validation (empty strings, invalid characters)

Potential path traversal security concerns for the implementation

marty-action · 2026-03-05T10:46:57Z

+
+export interface Directory extends FileSystemNode {
+  isDirectory: true;
+  children: FileSystemNode[];


The children array uses FileSystemNode[] but could be more precise with FileSystemItem[] to ensure type safety when iterating (currently requires type narrowing). Consider using FileSystemItem[] instead.

marty-action · 2026-03-05T10:47:14Z

Summary

This PR introduces core TypeScript interfaces for a virtual file system including FileSystemNode, File, Directory, FileSystem, and type guards isFile() and isDirectory(). The implementation is clean and follows TypeScript best practices.

Critical Issues

None - the implementation is solid for a foundational types PR.

Recommendations

Discriminated union pattern - Consider using a kind property instead of isDirectory: boolean for better type safety (prevents inconsistent states like File with isDirectory: true)
API documentation - The FileSystem interface methods lack documentation for edge cases (path existence, error handling, path validation)
Type precision - Directory.children could use FileSystemItem[] instead of FileSystemNode[] for better type inference
Test coverage - Consider adding edge case tests for null/undefined inputs to type guards

Positive Notes

Clean TypeScript interfaces with proper inheritance
Good use of type guards for runtime type narrowing
Tests are well-structured and cover the basic use cases
Exports are properly centralized through index.ts

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

marty-action · 2026-03-05T10:54:40Z

Code Review

Summary

The PR introduces core TypeScript interfaces for a virtual file system with proper type guards and tests. The implementation is clean and well-structured.

Critical Issues

None - the core types are well designed.

Recommendations

File size type (types.ts:12) - The size property uses number. Consider using bigint for large files or documenting the unit (bytes).
Missing path validation (types.ts:22-31) - The FileSystem interface defines method signatures but does not include path normalization or validation. Consider documenting expected path format.
Content type limitation (types.ts:11) - File content is limited to string. For binary file support, consider making content generic or using Uint8Array.
Missing async variants (types.ts:22-31) - All FileSystem methods are synchronous. Consider if async versions are needed for future implementations.

Positive Notes

Good use of TypeScript discriminated unions
Type guards use proper type predicates
Clean separation between FileSystemNode, File, and Directory
Tests are well-structured with good coverage of edge cases

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

marty-action · 2026-03-05T11:00:14Z

+describe("types", () => {
+  describe("isFile", () => {
+    it("returns true for File", () => {
+      const file: File = {


Test objects are missing required properties from FileSystemNode (name, path, createdAt, modifiedAt). While this works because the tests directory isn't included in TypeScript compilation, it would be better to include these properties for correctness and to serve as documentation.

marty-action · 2026-03-05T11:00:18Z

+
+export type FileSystemItem = File | Directory;
+
+export interface FileSystem {


Consider adding JSDoc comments to document: whether paths are absolute or relative, what the return values mean (e.g., what does rm returning boolean signify?), and error handling behavior for these methods.

marty-action · 2026-03-05T11:00:30Z

Review Summary

Summary: This PR introduces core TypeScript types for a virtual file system, including FileSystemNode, File, Directory, FileSystemItem unions, and a FileSystem interface with helper type guards. The types are well-structured and use proper TypeScript patterns like literal types for discrimination and type guards.

Critical Issues: None - this is a solid type definition PR.

Recommendations:

Add JSDoc comments to document the types, especially the FileSystem interface methods and the size property units (bytes)
Consider adding a FileSystemError type or documenting error handling behavior
The tests should include all required properties from FileSystemNode for correctness (see inline comment)
Consider adding a Readonly variant of these types if the filesystem will be exposed as immutable
The FileSystem interface could benefit from generic type parameters if you want to support different content types

Positive Notes:

Good use of discriminated union (isDirectory: false vs isDirectory: true) for type narrowing
Type guard functions (isFile, isDirectory) are well-implemented
Clean separation between base interface and specialized types
Proper use of Date type instead of string timestamps
Tests cover both positive and negative cases for type guards
Exports are clean and focused

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

marty-action · 2026-03-05T12:23:32Z

+  name: string;
+  path: string;
+  isDirectory: boolean;
+  createdAt: Date;


Consider using string (ISO date format) instead of Date for createdAt and modifiedAt. Date objects are not directly serializable to JSON, which could cause issues if the virtual file system needs to be persisted or transmitted.

marty-action · 2026-03-05T12:23:36Z

+  path: string;
+  isDirectory: boolean;
+  createdAt: Date;
+  modifiedAt: Date;


Consider using string (ISO date format) instead of Date for createdAt and modifiedAt. This ensures consistency and improves serializability.

marty-action · 2026-03-05T12:23:40Z

+export type File = FileSystemNode & {
+  isDirectory: false;
+  content: string;
+  size: number;


Consider documenting the unit for size (e.g., bytes) or creating a type alias to make it explicit.

marty-action · 2026-03-05T12:23:47Z

+
+export type FileSystemItem = File | Directory;
+
+export interface FileSystem {


The FileSystem interface defines method signatures but not their implementation. Consider documenting that this is intentionally an interface for the API contract, or if there's a default implementation class planned.

marty-action · 2026-03-05T12:23:57Z

Summary

This PR introduces core TypeScript interfaces for a virtual file system, including FileSystemNode, File, Directory, and FileSystem, along with type guards for runtime type checking.

Critical Issues

None identified

Recommendations

Date Serialization: Consider using string (ISO 8601 format) instead of Date for createdAt and modifiedAt fields. Date objects don't serialize to JSON directly, which could cause issues when persisting or transmitting the file system data.
Size Unit Documentation: Consider documenting that size is in bytes, or creating a type alias like type SizeInBytes = number for clarity.
FileSystem Interface: The FileSystem interface defines method signatures without implementation. This appears intentional for an API contract, but consider adding a JSDoc comment to clarify this design decision.

Positive Notes

Clean and well-structured type definitions using TypeScript's type system effectively
Discriminated union pattern (isDirectory: true/false) is well implemented
Type guards (isFile, isDirectory) are correctly implemented with type predicates
Good test coverage for the type guards with clear test cases
Tests are well-organized using vitest's describe blocks

- Use string (ISO 8601) instead of Date for serialization - Add SizeInBytes type alias for clarity - Add JSDoc for FileSystem interface Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

marty-action · 2026-03-05T12:34:32Z

+
+export type Directory = FileSystemNode & {
+  isDirectory: true;
+  children: FileSystemNode[];


The children array should be typed as FileSystemItem[] (which is File | Directory) rather than FileSystemNode[]. This ensures type safety when using the type guards (isFile, isDirectory) on children, since those guards expect FileSystemItem but will receive FileSystemNode.

marty-action · 2026-03-05T12:34:38Z

+  write(path: string, content: string): File;
+}
+
+export function isFile(item: FileSystemItem): item is File {


The type guards accept FileSystemItem but will be used on Directory.children which contains FileSystemNode items. Either widen the parameter type to FileSystemNode or ensure children are typed as FileSystemItem[] (see line 19).

marty-action · 2026-03-05T12:34:43Z

+  get(path: string): FileSystemItem | null;
+  mkdir(path: string): Directory;
+  touch(path: string, content?: string): File;
+  rm(path: string): boolean;


Consider if rm should return boolean or void. Returning boolean is unconventional for file deletion - typically it's either void (with exceptions on failure) or the deleted item. This could be confusing to API consumers.

marty-action · 2026-03-05T12:35:09Z

Review Summary

Positive Notes

Good use of discriminated union pattern with isDirectory property for type narrowing
Type guards (isFile, isDirectory) are correctly implemented as TypeScript type predicates
Well-documented FileSystem interface with clear API contract
Tests are well-structured and cover both type guard functions

Critical Issues

Type inconsistency: The Directory.children is typed as FileSystemNode[] but the type guards (isFile, isDirectory) accept FileSystemItem. This creates a type safety gap where children cannot be properly narrowed without explicit casting.

Recommendations

Consider using Date objects instead of ISO date strings for createdAt/modifiedAt - this would be more semantically correct and provide date utility methods
The rm(path: string): boolean return type is unconventional - consider using void or throwing exceptions on failure instead
Consider adding JSDoc comments to exported functions for better IDE support
Consider adding a FileSystemNode union type that represents the base type including the discriminator

Documentation

The PR description adequately covers the changes
Inline documentation on FileSystem interface is good
No README updates needed for this types-only PR

Overall: Good foundation for the virtual file system types. The main concern is the type consistency between FileSystemNode and FileSystemItem which should be addressed before merging.

- Use Date objects for createdAt/modifiedAt - Fix children type to FileSystemItem[] for type safety - Update type guards to accept FileSystemNode - Change rm() return type to void - Add JSDoc comments to all exports Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

marty-action · 2026-03-05T12:51:35Z

+export type FileSystemNode = {
+  name: string;
+  path: string;
+  isDirectory: boolean;


Consider removing isDirectory from FileSystemNode since it's redundant with the discriminated union pattern where File has isDirectory: false and Directory has isDirectory: true. The type guard already narrows correctly based on the subtype.

marty-action · 2026-03-05T12:51:41Z

@@ -0,0 +1,63 @@
+export type SizeInBytes = number;


Consider adding JSDoc for this type to document what SizeInBytes represents and any constraints (e.g., must be non-negative).

marty-action · 2026-03-05T12:51:52Z

@@ -0,0 +1,58 @@
+import { describe, it, expect } from "vitest";
+import { isFile, isDirectory, File, Directory } from "../src/types";


The test file is importing from ../src/types but the package.json tsconfig.json only includes src folder. The tests should either be in src/ or you need to update the tsconfig to include tests. This is not a bug since vitest handles this, but type-checking won't apply to tests.

marty-action · 2026-03-05T12:51:57Z

+/**
+ * Union type for file system items (files or directories).
+ */
+export type FileSystemItem = File | Directory;


Consider also exporting FileSystemItem since it's a useful union type that consumers may need for type annotations.

marty-action · 2026-03-05T12:52:02Z

+import { describe, it, expect } from "vitest";
+import { isFile, isDirectory, File, Directory } from "../src/types";
+
+describe("types", () => {


Consider adding tests for FileSystemItem union type to verify type narrowing works correctly with both File and Directory variants.

feat: define core types for virtual file system

c4d4318

- Add FileSystemNode, File, Directory interfaces - Add FileSystem interface with CRUD operations - Add isFile and isDirectory type guards - Add tests for type guards Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>