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

Make it work on Windows #748

Merged
merged 19 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/tiny-years-tie.md
Original file line number Diff line number Diff line change
@@ -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.
4 changes: 3 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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@v3
- name: Use Node.js ${{ matrix.node-version }}
Expand Down
16 changes: 9 additions & 7 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,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",
Expand Down
2 changes: 1 addition & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
Expand Down
4 changes: 3 additions & 1 deletion src/jest-dom/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ const extensions = ['.js', '.jsx', '.es6', '.es', '.mjs', '.ts', '.tsx'];

const stubs = {
[require.resolve('@testing-library/jest-dom/dist/to-have-style')]: `
export { toHaveStyle } from "${require.resolve('./to-have-style')}"
export { toHaveStyle } from ${JSON.stringify(
require.resolve('./to-have-style'),
)}
`,
// No need for polyfill in real browser
'css.escape': `
Expand Down
7 changes: 3 additions & 4 deletions src/module-server/bundle-npm-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,9 @@ 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;
return `export * from ${JSON.stringify(mod)}
export {${namedExports.join(', ')}} from ${JSON.stringify(mod)}
export { default } from ${JSON.stringify(mod)}`;
}
},
} as Plugin),
Expand Down
3 changes: 2 additions & 1 deletion src/module-server/extensions-and-detection.ts
Original file line number Diff line number Diff line change
@@ -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) =>
!(
Expand Down
17 changes: 12 additions & 5 deletions src/module-server/middleware/js.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -102,7 +102,14 @@ export const jsMiddleware = async ({
import.meta.pleasantestArgs = [...window._pleasantestArgs]
}`;
const fileSrc = await fs.readFile(file, 'utf8');
const inlineStartIdx = fileSrc.indexOf(code);
let EOL = '\n';
// 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') EOL = '\r\n';
const inlineStartIdx = fileSrc.indexOf(code.replaceAll('\n', EOL));
code = injectedArgsCode + code;
if (inlineStartIdx !== -1) {
const str = new MagicString(fileSrc);
Expand Down Expand Up @@ -180,16 +187,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);
}
Expand Down
10 changes: 7 additions & 3 deletions src/module-server/node-resolve.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down
9 changes: 6 additions & 3 deletions src/module-server/node-resolve.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -129,7 +129,7 @@ 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);
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);
Expand Down Expand Up @@ -161,7 +161,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)) {
Expand Down
9 changes: 7 additions & 2 deletions src/module-server/rollup-plugin-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
- Added object-hooks support and plugin ordering (from Vite version)
*/

import { promises as fs } from 'node:fs';
import { dirname, resolve } from 'node:path';

import type { DecodedSourceMap, RawSourceMap } from '@ampproject/remapping';
Expand Down Expand Up @@ -282,7 +283,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;
Expand All @@ -304,7 +305,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;
}
}

Expand Down
9 changes: 7 additions & 2 deletions src/module-server/transform-imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}
Expand Down
15 changes: 12 additions & 3 deletions tests/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -48,12 +50,19 @@ 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');
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:###:###')
.replace(
fileRegex,
(_match, relativePath) =>
`<root>${relativePath.replaceAll(path.sep, '/')}:###:###`,
)
);
};

Expand Down
Loading