refactor: improve result pattern responses

.changeset/ninety-lights-occur.md

@@ -0,0 +1,5 @@
+---
+"@webiny/stdlib": patch
+---
+
+Add BrowserWindow abstraction to decouple browser features from the global `window` object, with real and null implementations. Replace silent `void` returns with `Result` pattern across `DirectoryTool.create`, `FileTool.writeFile`/`copy`, `JsonFileTool.writeJson`, and `PackageJsonFileTool.write`. Add `createOrThrow` to DirectoryTool. New typed errors: `DirectoryCreateError`, `FileWriteError`, `FileCopyError`.

README.md

@@ -58,10 +58,11 @@ The package is ESM-only and ships three subpath exports. Because each is a separ
 
 ## `@webiny/stdlib/browser` — Browser
 
-| Feature                    | Description                                                                                                       |
-| -------------------------- | ----------------------------------------------------------------------------------------------------------------- |
-| `LocalStorageCacheFeature` | `Cache` implementation backed by `window.localStorage` — [docs](src/browser/features/LocalStorageCache/README.md) |
-| `BrowserEnvFeature`        | `Env` implementation backed by an injected variables object — [docs](src/browser/features/BrowserEnv/README.md)   |
+| Feature                    | Description                                                                                                                            |
+| -------------------------- | -------------------------------------------------------------------------------------------------------------------------------------- |
+| `BrowserWindowFeature`     | Abstraction over the browser `window` object with real and null implementations — [docs](src/browser/features/BrowserWindow/README.md) |
+| `LocalStorageCacheFeature` | `Cache` implementation backed by `window.localStorage` — [docs](src/browser/features/LocalStorageCache/README.md)                      |
+| `BrowserEnvFeature`        | `Env` implementation backed by an injected variables object — [docs](src/browser/features/BrowserEnv/README.md)                        |
 
 ---
 

__tests__/browser/BrowserWindow.test.ts

@@ -0,0 +1,58 @@
+// @vitest-environment happy-dom
+import { Container } from "@webiny/di";
+import { describe, it, expect } from "vitest";
+import { BrowserWindow } from "../../src/browser/features/BrowserWindow/abstractions/BrowserWindow.js";
+import {
+    BrowserWindow as BrowserWindowImpl,
+    createBrowserWindow
+} from "../../src/browser/features/BrowserWindow/BrowserWindow.js";
+import {
+    NullBrowserWindow,
+    createNullBrowserWindow
+} from "../../src/browser/features/BrowserWindow/NullBrowserWindow.js";
+import { BrowserWindowFeature } from "../../src/browser/features/BrowserWindow/feature.js";
+import { NullBrowserWindowFeature } from "../../src/browser/features/BrowserWindow/nullFeature.js";
+
+describe("BrowserWindow", () => {
+    describe("real implementation", () => {
+        it("exposes localStorage from the global window", () => {
+            const container = new Container();
+            container.register(BrowserWindowImpl).inSingletonScope();
+            const bw = container.resolve(BrowserWindow);
+            expect(bw.localStorage).toBe(window.localStorage);
+        });
+
+        it("resolves via BrowserWindowFeature", () => {
+            const container = new Container();
+            BrowserWindowFeature.register(container);
+            const bw = container.resolve(BrowserWindow);
+            expect(bw.localStorage).toBe(window.localStorage);
+        });
+
+        it("creates via factory function", () => {
+            const bw = createBrowserWindow();
+            expect(bw.localStorage).toBe(window.localStorage);
+        });
+    });
+
+    describe("null implementation", () => {
+        it("returns null for localStorage", () => {
+            const container = new Container();
+            container.register(NullBrowserWindow).inSingletonScope();
+            const bw = container.resolve(BrowserWindow);
+            expect(bw.localStorage).toBeNull();
+        });
+
+        it("resolves via NullBrowserWindowFeature", () => {
+            const container = new Container();
+            NullBrowserWindowFeature.register(container);
+            const bw = container.resolve(BrowserWindow);
+            expect(bw.localStorage).toBeNull();
+        });
+
+        it("creates via factory function", () => {
+            const bw = createNullBrowserWindow();
+            expect(bw.localStorage).toBeNull();
+        });
+    });
+});

__tests__/browser/LocalStorageCache.test.ts

@@ -1,6 +1,7 @@
 // @vitest-environment happy-dom
 import { Container } from "@webiny/di";
 import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
+import { BrowserWindow as BrowserWindowImpl } from "../../src/browser/features/BrowserWindow/BrowserWindow.js";
 import { LocalStorageCache } from "../../src/browser/features/LocalStorageCache/LocalStorageCache.js";
 import { Cache } from "../../src/index.js";
 import {
@@ -11,6 +12,7 @@ import {
 
 function makeCache(): Cache.Interface {
     const container = new Container();
+    container.register(BrowserWindowImpl).inSingletonScope();
     container.register(LocalStorageCache).inSingletonScope();
     return container.resolve(Cache);
 }

__tests__/node/DirectoryTool.test.ts

@@ -1,10 +1,11 @@
 import { afterEach, beforeEach, describe, expect, it } from "vitest";
-import { existsSync, mkdirSync, rmSync, writeFileSync } from "node:fs";
+import { existsSync, mkdirSync, rmSync, writeFileSync, chmodSync } from "node:fs";
 import { join } from "node:path";
 import { tmpdir } from "node:os";
 import { Container } from "@webiny/di";
 import {
     DirectoryTool,
+    DirectoryCreateError,
     DirectoryToolFeature,
     createDirectoryTool
 } from "../../src/node/features/DirectoryTool/index.js";
@@ -45,21 +46,58 @@ describe("DirectoryTool", () => {
     });
 
     describe("create", () => {
-        it("creates a new directory", () => {
+        it("creates a new directory and returns ok", () => {
             const dir = join(tmpDir, "new-dir");
-            tool.create(dir);
+            const result = tool.create(dir);
+            expect(result.isOk()).toBe(true);
             expect(existsSync(dir)).toBe(true);
         });
 
         it("creates nested directories", () => {
             const dir = join(tmpDir, "a", "b", "c");
-            tool.create(dir);
+            const result = tool.create(dir);
+            expect(result.isOk()).toBe(true);
             expect(existsSync(dir)).toBe(true);
         });
 
         it("is idempotent on existing directories", () => {
-            tool.create(tmpDir);
-            expect(() => tool.create(tmpDir)).not.toThrow();
+            const result = tool.create(tmpDir);
+            expect(result.isOk()).toBe(true);
+        });
+
+        it("returns a failure Result when creation is impossible", () => {
+            const blocked = join(tmpDir, "blocked");
+            mkdirSync(blocked, { mode: 0o000 });
+            try {
+                const result = tool.create(join(blocked, "child", "deep"));
+                expect(result.isFail()).toBe(true);
+                if (result.isFail()) {
+                    expect(result.error).toBeInstanceOf(DirectoryCreateError);
+                    expect(result.error.data.path).toBe(join(blocked, "child", "deep"));
+                }
+            } finally {
+                chmodSync(blocked, 0o755);
+            }
+        });
+    });
+
+    describe("createOrThrow", () => {
+        it("creates a new directory without throwing", () => {
+            const dir = join(tmpDir, "new-dir-throw");
+            tool.createOrThrow(dir);
+            expect(existsSync(dir)).toBe(true);
+        });
+
+        it("throws DirectoryCreateError when creation is impossible", () => {
+            const blocked = join(tmpDir, "blocked");
+            mkdirSync(blocked, { mode: 0o000 });
+            try {
+                expect(() => tool.createOrThrow(join(blocked, "child", "deep"))).toThrow(
+                    DirectoryCreateError
+                );
+            } finally {
+                chmodSync(blocked, 0o755);
+            }
         });
     });
 
@@ -115,12 +153,40 @@ describe("DirectoryTool", () => {
         it("does not throw when source is missing", () => {
             expect(() => tool.copy(join(tmpDir, "missing"), join(tmpDir, "dest"))).not.toThrow();
         });
+
+        it("does not throw when target directory cannot be created", () => {
+            const src = join(tmpDir, "src");
+            mkdirSync(src, { recursive: true });
+            writeFileSync(join(src, "file.txt"), "content");
+            const blocked = join(tmpDir, "blocked");
+            mkdirSync(blocked, { mode: 0o000 });
+            try {
+                expect(() => tool.copy(src, join(blocked, "child", "dest"))).not.toThrow();
+            } finally {
+                chmodSync(blocked, 0o755);
+            }
+        });
     });
 
     describe("copyOrThrow", () => {
         it("throws when source is missing", () => {
             expect(() => tool.copyOrThrow(join(tmpDir, "missing"), join(tmpDir, "dest"))).toThrow();
         });
+
+        it("throws when target directory cannot be created", () => {
+            const src = join(tmpDir, "src");
+            mkdirSync(src, { recursive: true });
+            writeFileSync(join(src, "file.txt"), "content");
+            const blocked = join(tmpDir, "blocked");
+            mkdirSync(blocked, { mode: 0o000 });
+            try {
+                expect(() => tool.copyOrThrow(src, join(blocked, "child", "dest"))).toThrow(
+                    DirectoryCreateError
+                );
+            } finally {
+                chmodSync(blocked, 0o755);
+            }
+        });
     });
 
     describe("glob", () => {
@@ -198,7 +264,8 @@ describe("createDirectoryTool", () => {
     it("creates a working tool without arguments", () => {
         const tool = createDirectoryTool();
         const dir = join(tmpDir, "factory-dir");
-        tool.create(dir);
+        const result = tool.create(dir);
+        expect(result.isOk()).toBe(true);
         expect(existsSync(dir)).toBe(true);
     });
 

__tests__/node/FileTool.test.ts

@@ -1,10 +1,12 @@
 import { afterEach, beforeEach, describe, expect, it } from "vitest";
-import { existsSync, mkdirSync, rmSync, writeFileSync } from "node:fs";
+import { existsSync, mkdirSync, rmSync, writeFileSync, chmodSync } from "node:fs";
 import { join } from "node:path";
 import { tmpdir } from "node:os";
 import { Container } from "@webiny/di";
 import {
     FileTool,
+    FileWriteError,
+    FileCopyError,
     FileToolFeature,
     createFileTool
 } from "../../src/node/features/FileTool/index.js";
@@ -76,24 +78,43 @@ describe("FileTool", () => {
     });
 
     describe("writeFile", () => {
-        it("creates a file with the given content", () => {
+        it("returns ok and creates a file with the given content", () => {
             const file = join(tmpDir, "new.txt");
-            tool.writeFile(file, "written");
+            const result = tool.writeFile(file, "written");
+            expect(result.isOk()).toBe(true);
             expect(tool.readFile(file)).toBe("written");
         });
 
         it("creates parent directories as needed", () => {
             const file = join(tmpDir, "nested", "deep", "file.txt");
-            tool.writeFile(file, "deep content");
+            const result = tool.writeFile(file, "deep content");
+            expect(result.isOk()).toBe(true);
             expect(existsSync(file)).toBe(true);
         });
 
         it("overwrites existing content", () => {
             const file = join(tmpDir, "file.txt");
             writeFileSync(file, "old");
-            tool.writeFile(file, "new");
+            const result = tool.writeFile(file, "new");
+            expect(result.isOk()).toBe(true);
             expect(tool.readFile(file)).toBe("new");
         });
+
+        it("returns a failure Result when parent directory cannot be created", () => {
+            const blocked = join(tmpDir, "blocked");
+            mkdirSync(blocked, { mode: 0o000 });
+            try {
+                const file = join(blocked, "child", "file.txt");
+                const result = tool.writeFile(file, "x");
+                expect(result.isFail()).toBe(true);
+                if (result.isFail()) {
+                    expect(result.error).toBeInstanceOf(FileWriteError);
+                    expect(result.error.data.path).toBe(file);
+                }
+            } finally {
+                chmodSync(blocked, 0o755);
+            }
+        });
     });
 
     describe("writeFileOrThrow", () => {
@@ -102,6 +123,18 @@ describe("FileTool", () => {
             tool.writeFileOrThrow(file, "content");
             expect(tool.readFile(file)).toBe("content");
         });
+
+        it("throws when parent directory cannot be created", () => {
+            const blocked = join(tmpDir, "blocked");
+            mkdirSync(blocked, { mode: 0o000 });
+            try {
+                expect(() =>
+                    tool.writeFileOrThrow(join(blocked, "child", "file.txt"), "x")
+                ).toThrow();
+            } finally {
+                chmodSync(blocked, 0o755);
+            }
+        });
     });
 
     describe("remove", () => {
@@ -118,26 +151,47 @@ describe("FileTool", () => {
     });
 
     describe("copy", () => {
-        it("duplicates a file", () => {
+        it("returns ok and duplicates a file", () => {
             const src = join(tmpDir, "src.txt");
             const dest = join(tmpDir, "dest.txt");
             writeFileSync(src, "content");
-            tool.copy(src, dest);
+            const result = tool.copy(src, dest);
+            expect(result.isOk()).toBe(true);
             expect(tool.readFile(dest)).toBe("content");
         });
 
         it("creates parent directories for the destination", () => {
             const src = join(tmpDir, "src.txt");
             const dest = join(tmpDir, "nested", "dest.txt");
             writeFileSync(src, "content");
-            tool.copy(src, dest);
+            const result = tool.copy(src, dest);
+            expect(result.isOk()).toBe(true);
             expect(existsSync(dest)).toBe(true);
         });
 
-        it("does not throw when source is missing", () => {
-            expect(() =>
-                tool.copy(join(tmpDir, "missing.txt"), join(tmpDir, "dest.txt"))
-            ).not.toThrow();
+        it("returns a failure Result when source is missing", () => {
+            const result = tool.copy(join(tmpDir, "missing.txt"), join(tmpDir, "dest.txt"));
+            expect(result.isFail()).toBe(true);
+            if (result.isFail()) {
+                expect(result.error).toBeInstanceOf(FileCopyError);
+                expect(result.error.data.source).toBe(join(tmpDir, "missing.txt"));
+            }
+        });
+
+        it("returns a failure Result when destination directory cannot be created", () => {
+            const src = join(tmpDir, "src.txt");
+            writeFileSync(src, "content");
+            const blocked = join(tmpDir, "blocked");
+            mkdirSync(blocked, { mode: 0o000 });
+            try {
+                const result = tool.copy(src, join(blocked, "child", "dest.txt"));
+                expect(result.isFail()).toBe(true);
+                if (result.isFail()) {
+                    expect(result.error).toBeInstanceOf(FileCopyError);
+                }
+            } finally {
+                chmodSync(blocked, 0o755);
+            }
         });
     });
 
@@ -147,6 +201,18 @@ describe("FileTool", () => {
                 tool.copyOrThrow(join(tmpDir, "missing.txt"), join(tmpDir, "dest.txt"))
             ).toThrow();
         });
+
+        it("throws when destination directory cannot be created", () => {
+            const src = join(tmpDir, "src.txt");
+            writeFileSync(src, "content");
+            const blocked = join(tmpDir, "blocked");
+            mkdirSync(blocked, { mode: 0o000 });
+            try {
+                expect(() => tool.copyOrThrow(src, join(blocked, "child", "dest.txt"))).toThrow();
+            } finally {
+                chmodSync(blocked, 0o755);
+            }
+        });
     });
 });
 
@@ -165,7 +231,8 @@ describe("createFileTool", () => {
     it("creates a working tool without arguments", () => {
         const tool = createFileTool();
         const file = join(tmpDir, "factory.txt");
-        tool.writeFile(file, "hello");
+        const result = tool.writeFile(file, "hello");
+        expect(result.isOk()).toBe(true);
         expect(tool.readFile(file)).toBe("hello");
     });
 
@@ -190,7 +257,8 @@ describe("createFileTool", () => {
         const directoryTool = createDirectoryTool();
         const tool = createFileTool({ directoryTool });
         const file = join(tmpDir, "nested", "custom.txt");
-        tool.writeFile(file, "content");
+        const result = tool.writeFile(file, "content");
+        expect(result.isOk()).toBe(true);
         expect(tool.readFile(file)).toBe("content");
     });
 });