From 34b605d0a8652f24b1da061541503af436f568d6 Mon Sep 17 00:00:00 2001 From: sidgaikwad Date: Fri, 4 Sep 2026 12:14:07 +0530 Subject: [PATCH 1/2] test: cover the SSR and 'use client' guarantees The README states the component works in React Server Components environments, and that it touches the DOM only inside effects. Nothing verified either claim: the whole suite runs in jsdom, and the 'use client' banner is a tsup config detail that a bundler or config change could silently drop with every existing test still passing. Add test/ssr.test.tsx, which runs with `@vitest-environment node`: - asserts there really is no DOM in that environment, so the rest of the file proves something - renders the component with renderToString, bare and with every prop set - asserts both built bundles start with 'use client' The dist assertion is skipped when dist/ is absent, so it runs in the CI job that builds first and does not fail the Node/React matrix jobs. --- test/ssr.test.tsx | 62 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 test/ssr.test.tsx diff --git a/test/ssr.test.tsx b/test/ssr.test.tsx new file mode 100644 index 0000000..19d5f82 --- /dev/null +++ b/test/ssr.test.tsx @@ -0,0 +1,62 @@ +// @vitest-environment node +// +// The rest of the suite runs in jsdom, so nothing here verified the +// README's claim that the component "works out of the box in React Server +// Components environments ... and touches the DOM only inside effects". +// This file runs with no DOM at all. +import { existsSync, readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; + +import { renderToString } from 'react-dom/server'; + +import ImageEditor from '../src'; + +it('has no DOM in this environment', () => { + // Guards the guard: if jsdom leaked in, the test below proves nothing. + expect(typeof document).toBe('undefined'); + expect(typeof window).toBe('undefined'); +}); + +it('renders to a string on the server without touching the DOM', () => { + const html = renderToString( + + ); + + expect(html).toContain('id="ssr"'); +}); + +it('renders on the server with every prop set', () => { + // Effects never run on the server, so a prop that reaches the DOM during + // render rather than in an effect would throw here. + expect(() => + renderToString( + {}} + onSave={() => {}} + onCancel={() => {}} + onLoadError={() => {}} + onError={() => {}} + /> + ) + ).not.toThrow(); +}); + +// The 'use client' banner comes from tsup config, which nothing else +// checks — a bundler or config change could silently drop it and every +// existing test would still pass. Runs after `npm run build`; skipped in +// the React/Node matrix jobs, which do not build. +const dist = (file: string) => resolve(__dirname, '..', 'dist', file); +const built = existsSync(dist('index.mjs')); + +it.skipIf(!built)('ships the "use client" directive in both builds', () => { + for (const file of ['index.js', 'index.mjs']) { + expect(readFileSync(dist(file), 'utf8').startsWith("'use client';")).toBe( + true + ); + } +}); From 05f3449cf0b1e6f5ebd91e9b2388552affe49136 Mon Sep 17 00:00:00 2001 From: sidgaikwad Date: Sat, 12 Sep 2026 22:04:33 +0530 Subject: [PATCH 2/2] test: declare @types/node, and correct the build comment The SSR test imports node:fs and node:path and reads __dirname, but @types/node was never declared, so typecheck failed in CI on all three. It passed locally only by accident: TypeScript walks up the directory tree for node_modules/@types and had been picking up a stray @types/node in the developer's home directory. The project's own node_modules had no copy, so nothing about the local pass was reproducible. Pin @types/node to 24.10.9, matching the demo's existing pin, and add "node" to compilerOptions.types so the dependency is explicit rather than inherited from wherever the checkout happens to sit. Also corrects the comment above the dist assertion. It claimed the matrix jobs do not build, but npm ci runs the prepare script and prepare builds, so dist/ exists in every CI job and the assertion is not skipped there. The skipIf guard only covers a local run before anyone has built. --- package-lock.json | 18 ++++++++++++++++++ package.json | 1 + test/ssr.test.tsx | 5 +++-- tsconfig.json | 2 +- 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 602a0a0..75c3112 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14,6 +14,7 @@ "devDependencies": { "@eslint/js": "10.0.1", "@testing-library/react": "16.3.2", + "@types/node": "24.10.9", "@types/react": "19.2.17", "@types/react-dom": "19.2.3", "@vitest/coverage-v8": "4.1.10", @@ -2026,6 +2027,16 @@ "dev": true, "license": "MIT" }, + "node_modules/@types/node": { + "version": "24.10.9", + "resolved": "https://registry.npmjs.org/@types/node/-/node-24.10.9.tgz", + "integrity": "sha512-ne4A0IpG3+2ETuREInjPNhUGis1SFjv1d5asp8MzEAGtOZeTeHVDOYqOgqfhvseqg/iXty2hjBf1zAOb7RNiNw==", + "dev": true, + "license": "MIT", + "dependencies": { + "undici-types": "~7.16.0" + } + }, "node_modules/@types/react": { "version": "19.2.17", "resolved": "https://registry.npmjs.org/@types/react/-/react-19.2.17.tgz", @@ -4854,6 +4865,13 @@ "node": ">=20.18.1" } }, + "node_modules/undici-types": { + "version": "7.16.0", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.16.0.tgz", + "integrity": "sha512-Zz+aZWSj8LE6zoxD+xrjh4VfkIG8Ya6LvYkZqtUQGJPZjYl53ypCaUwWqo7eI0x66KBGeRo+mlBEkMSeSZ38Nw==", + "dev": true, + "license": "MIT" + }, "node_modules/update-browserslist-db": { "version": "1.3.2", "resolved": "https://registry.npmjs.org/update-browserslist-db/-/update-browserslist-db-1.3.2.tgz", diff --git a/package.json b/package.json index 0f93437..c34572b 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,7 @@ "devDependencies": { "@eslint/js": "10.0.1", "@testing-library/react": "16.3.2", + "@types/node": "24.10.9", "@types/react": "19.2.17", "@types/react-dom": "19.2.3", "@vitest/coverage-v8": "4.1.10", diff --git a/test/ssr.test.tsx b/test/ssr.test.tsx index 19d5f82..b562ce3 100644 --- a/test/ssr.test.tsx +++ b/test/ssr.test.tsx @@ -48,8 +48,9 @@ it('renders on the server with every prop set', () => { // The 'use client' banner comes from tsup config, which nothing else // checks — a bundler or config change could silently drop it and every -// existing test would still pass. Runs after `npm run build`; skipped in -// the React/Node matrix jobs, which do not build. +// existing test would still pass. dist/ exists in every CI job, because +// `npm ci` runs the `prepare` script and that builds; the skip is only for +// a local run before anyone has built. const dist = (file: string) => resolve(__dirname, '..', 'dist', file); const built = existsSync(dist('index.mjs')); diff --git a/tsconfig.json b/tsconfig.json index 0bc2c11..9ed99bf 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -19,6 +19,6 @@ "forceConsistentCasingInFileNames": true, "noEmit": true, "resolveJsonModule": true, - "types": ["vitest/globals"] + "types": ["vitest/globals", "node"] } }