Make it work on Windows (#748)

.changeset/tiny-years-tie.md

@@ -0,0 +1,7 @@
+---
+'pleasantest': minor
+---
+
+Reimplement windows support
+
+Long ago, Pleasantest worked on Windows, but without regular testing it gradually diverged. This release adds proper Windows support back and adds automated testing for it.

.github/workflows/ci.yml

@@ -12,10 +12,12 @@ concurrency:
 
 jobs:
   test:
-    runs-on: ubuntu-latest
+    runs-on: ${{ matrix.platform }}
     strategy:
+      fail-fast: false
       matrix:
         node-version: [18.x, 20.x, 22.x]
+        platform: [ubuntu-latest, windows-latest]
     steps:
       - uses: actions/checkout@v4
       - name: Use Node.js ${{ matrix.node-version }}

package.json

@@ -117,13 +117,15 @@
   },
   "main": "./dist/cjs/index.cjs",
   "exports": {
-    "require": {
-      ".": "./dist/cjs/index.cjs",
-      "types": "./dist/index.d.cts"
-    },
-    "import": {
-      ".": "./dist/esm/index.mjs",
-      "types": "./dist/index.d.mts"
+    ".": {
+      "require": {
+        "default": "./dist/cjs/index.cjs",
+        "types": "./dist/index.d.cts"
+      },
+      "import": {
+        "default": "./dist/esm/index.mjs",
+        "types": "./dist/index.d.mts"
+      }
     }
   },
   "types": "./dist/index.d.ts",

src/index.ts

@@ -124,7 +124,7 @@ export const withBrowser: WithBrowser = (...args: any[]) => {
     // ignore if it is the current file
     if (stackItem === thisFile) return false;
     // ignore if it is an internal-to-node thing
-    if (!stackItem.startsWith('/')) return false;
+    if (stackItem.startsWith('node:')) return false;
     // Find the first item that is not the current file
     return true;
   });

src/jest-dom/rollup.config.js

@@ -1,19 +1,22 @@
-import * as childProcess from 'node:child_process';
-import { promisify } from 'node:util';
+import { fork } from 'node:child_process';
 
 import babel from '@rollup/plugin-babel';
 import nodeResolve from '@rollup/plugin-node-resolve';
 import terser from '@rollup/plugin-terser';
 
 import { rollupPluginDomAccessibilityApi } from '../rollup-plugin-dom-accessibility-api.js';
 
-const exec = promisify(childProcess.exec);
-
 const extensions = ['.js', '.jsx', '.es6', '.es', '.mjs', '.ts', '.tsx'];
 
-const { stdout, stderr } = await exec('./node_modules/.bin/patch-package');
-process.stdout.write(stdout);
-process.stderr.write(stderr);
+await new Promise((resolve, reject) => {
+  const child = fork('./node_modules/patch-package/index.js', [], {
+    stdio: 'inherit',
+  });
+  child.on('exit', (code) => {
+    if (code === 0) resolve();
+    else reject(new Error(`patch-package exited with code ${code}`));
+  });
+});
 
 const stubs = {
   chalk: `

src/module-server/bundle-npm-module.ts

@@ -70,10 +70,10 @@ export const bundleNpmModule = async (
           },
           load(id) {
             if (id === virtualEntry) {
-              const code = `export * from '${mod}'
-export {${namedExports.join(', ')}} from '${mod}'
-export { default } from '${mod}'`;
-              return code;
+              const modNameString = JSON.stringify(mod);
+              return `export * from ${modNameString}
+              export {${namedExports.join(', ')}} from ${modNameString}
+              export { default } from ${modNameString}`;
             }
           },
         } as Plugin),

src/module-server/extensions-and-detection.ts

@@ -1,11 +1,12 @@
+import { isAbsolute } from 'node:path';
 export const npmPrefix = '@npm/';
 
 export const isRelativeOrAbsoluteImport = (id: string) =>
   id === '.' ||
   id === '..' ||
   id.startsWith('./') ||
   id.startsWith('../') ||
-  id.startsWith('/');
+  isAbsolute(id);
 
 export const isBareImport = (id: string) =>
   !(

src/module-server/middleware/js.ts

@@ -1,5 +1,5 @@
 import { promises as fs } from 'node:fs';
-import { dirname, posix, relative, resolve, sep } from 'node:path';
+import { dirname, isAbsolute, posix, relative, resolve, sep } from 'node:path';
 
 import type { DecodedSourceMap, RawSourceMap } from '@ampproject/remapping';
 import MagicString from 'magic-string';
@@ -102,7 +102,8 @@ export const jsMiddleware = async ({
           import.meta.pleasantestArgs = [...window._pleasantestArgs]
         }`;
         const fileSrc = await fs.readFile(file, 'utf8');
-        const inlineStartIdx = fileSrc.indexOf(code);
+        const EOL = detectLineEndings(fileSrc);
+        const inlineStartIdx = fileSrc.indexOf(code.replaceAll('\n', EOL));
         code = injectedArgsCode + code;
         if (inlineStartIdx !== -1) {
           const str = new MagicString(fileSrc);
@@ -180,16 +181,16 @@ export const jsMiddleware = async ({
           if (resolved) {
             spec = typeof resolved === 'object' ? resolved.id : resolved;
             if (spec.startsWith('@npm/')) return addBuildId(`/${spec}`);
-            if (/^(\/|\\|[a-z]:\\)/i.test(spec)) {
+            if (isAbsolute(path)) {
               // Change FS-absolute paths to relative
-              spec = relative(dirname(file), spec).split(sep).join(posix.sep);
+              spec = relative(dirname(file), spec).split(sep).join('/');
               if (!/^\.?\.?\//.test(spec)) spec = `./${spec}`;
             }
 
             if (typeof resolved === 'object' && resolved.external) {
               if (/^(data|https?):/.test(spec)) return spec;
 
-              spec = relative(root, spec).split(sep).join(posix.sep);
+              spec = relative(root, spec).split(sep).join('/');
               if (!/^(\/|[\w-]+:)/.test(spec)) spec = `/${spec}`;
               return addBuildId(spec);
             }
@@ -233,3 +234,13 @@ export const jsMiddleware = async ({
     }
   };
 };
+
+const detectLineEndings = (fileSrc: string) => {
+  // Find the first line end (\n) and check if the character before is \r
+  // This tells us whether the file uses \r\n or just \n for line endings.
+  // Using node:os.EOL is not sufficient because git on windows
+  // Can be configured to check out files with either kind of line ending.
+  const firstLineEndPos = fileSrc.indexOf('\n');
+  if (fileSrc[firstLineEndPos - 1] === '\r') return '\r\n';
+  return '\n';
+};

src/module-server/node-resolve.test.ts

@@ -41,18 +41,22 @@ const createFs = async (input: string) => {
   );
 
   /** Replaces all instances of randomized tmp dir with "." */
-  const unrandomizePath = (text: string) => text.split(dir).join('.');
+  const unrandomizePath = (text: string) => text.replaceAll(dir, '.');
 
   const resolve = async (id: string, { from }: { from?: string } = {}) => {
     const result = await nodeResolve(
       id,
       join(dir, from || 'index.js'),
       dir,
     ).catch((error) => {
-      throw changeErrorMessage(error, (error) => unrandomizePath(error));
+      throw changeErrorMessage(error, (error) =>
+        unrandomizePath(error).replaceAll(sep, '/'),
+      );
     });
     if (result)
-      return unrandomizePath(typeof result === 'string' ? result : result.path);
+      return unrandomizePath(
+        typeof result === 'string' ? result : result.path,
+      ).replaceAll(sep, '/');
   };
 
   return { resolve };

src/module-server/node-resolve.ts

@@ -1,5 +1,5 @@
 import { promises as fs } from 'node:fs';
-import { dirname, join, resolve as pResolve, posix, relative } from 'node:path';
+import { dirname, join, resolve as pResolve, relative, sep } from 'node:path';
 
 import { resolve, legacy as resolveLegacy } from 'resolve.exports';
 
@@ -129,7 +129,8 @@ export const resolveFromNodeModules = async (
   const cacheKey = resolveCacheKey(id, importer, root);
   const cached = resolveCache.get(cacheKey);
   if (cached) return cached;
-  const pathChunks = id.split(posix.sep);
+  // Split the path up into chunks based on either kind of slash
+  const pathChunks = id.split(/[/\\]/g);
   const isNpmNamespace = id[0] === '@';
   // If it is an npm namespace, then get the first two folders, otherwise just one
   const packageName = pathChunks.slice(0, isNpmNamespace ? 2 : 1);
@@ -161,7 +162,10 @@ export const resolveFromNodeModules = async (
   }
 
   const pkgJson = await readPkgJson(pkgDir);
-  const main = readMainFields(pkgJson, subPath, true);
+  // Main/exports fields in package.json are defined with forward slashes.
+  // On windows, subPath will have \ instead of /, but we need to change it
+  // to match what will be listed in the package.json.
+  const main = readMainFields(pkgJson, subPath.replaceAll(sep, '/'), true);
   let result;
   if (main) result = join(pkgDir, main);
   else if (!('exports' in pkgJson)) {

src/module-server/rollup-plugin-container.ts

@@ -65,6 +65,7 @@
   - Updated to use import { parseAst } from 'rollup/parseAst' instead of acorn (rollup v4 change)
   */
 
+import { promises as fs } from 'node:fs';
 import { dirname, resolve } from 'node:path';
 
 import type { DecodedSourceMap, RawSourceMap } from '@ampproject/remapping';
@@ -279,7 +280,7 @@ export const createPluginContainer = (plugins: Plugin[]) => {
           }
         } catch (error) {
           if (error instanceof ErrorWithLocation) {
-            if (!error.filename) error.filename = id;
+            if (!error.filename) error.filename = await fs.realpath(id);
             // If the error has a location,
             // apply the source maps to get the original location
             const line = error.line;
@@ -301,7 +302,11 @@ export const createPluginContainer = (plugins: Plugin[]) => {
                     ? undefined
                     : sourceLocation.column;
               }
-              error.filename = sourceLocation.source || id;
+              // Source map filenames get URI encoded
+              error.filename = sourceLocation.source
+                ? // Fix path slashes (windows), drive capitalization (windows)
+                  await fs.realpath(decodeURIComponent(sourceLocation.source))
+                : id;
             }
           }
 

src/module-server/transform-imports.ts

@@ -30,6 +30,7 @@
   - Parsing errors are thrown with code frame
   */
 
+import { promises as fs } from 'node:fs';
 import { extname } from 'node:path';
 
 import type { DecodedSourceMap, RawSourceMap } from '@ampproject/remapping';
@@ -77,7 +78,7 @@ export const transformImports = async (
       message: `Error parsing module with es-module-lexer.${suggestion}`,
       line,
       column,
-      filename: id,
+      filename: await fs.realpath(id),
     });
 
     if (map) {
@@ -90,7 +91,11 @@ export const transformImports = async (
         modifiedError.column =
           sourceLocation.column === null ? undefined : sourceLocation.column;
       }
-      modifiedError.filename = sourceLocation.source || id;
+      // Source map filenames get URI encoded
+      modifiedError.filename = sourceLocation.source
+        ? // Fix path slashes (windows), drive capitalization (windows)
+          await fs.realpath(decodeURIComponent(sourceLocation.source))
+        : id;
     }
     throw modifiedError;
   }

tests/test-utils.ts

@@ -23,7 +23,9 @@ export const printErrorFrames = async (error?: Error) => {
           return stackFrame.raw;
         }
 
-        const relativePath = path.relative(process.cwd(), stackFrame.fileName);
+        const relativePath = path
+          .relative(process.cwd(), stackFrame.fileName)
+          .replaceAll(path.sep, '/');
         if (relativePath.startsWith('dist/')) return relativePath;
         let file;
         try {
@@ -48,12 +50,25 @@ const stripAnsi = (input: string) => input.replace(ansiRegex(), '');
 
 const removeLineNumbers = (input: string) => {
   const lineRegex = /^\s*▶?\s*(\d)*\s+│/gm;
-  const fileRegex = new RegExp(`${process.cwd()}([a-zA-Z/._-]*)[\\d:]*`, 'g');
+  // Creates a regex for the current directory.
+  // Backslashes need to be escaped for use in the regex.
+  // The [\\d:] part (double escaped because it is in a string) allows digits or colons
+  // (to match the line/column number)
+  const fileRegex = new RegExp(
+    `${process.cwd().replaceAll('\\', '\\\\')}([a-zA-Z/\\\\._-]*)[\\d:]*`,
+    'g',
+  );
   return (
     input
       .replace(lineRegex, (_match, lineNum) => (lineNum ? ' ### │' : '     │'))
       // Take out the file paths so the tests will pass on more than 1 person's machine
-      .replace(fileRegex, '<root>$1:###:###')
+      // Take out the line numbers so that code changes shifting the line numbers
+      // don't break the tests
+      .replace(
+        fileRegex,
+        (_match, relativePath) =>
+          `<root>${relativePath.replaceAll(path.sep, '/')}:###:###`,
+      )
   );
 };