Run vitest 4.1.11 in the browser shell - #349
vanilla-wave wants to merge 4 commits into
Conversation
Bare npm overrides, the Node surfaces that package links, and both test pools now report pass/fail counts and Node's exit code. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Netlify preview: https://pr-349--rifty-playground.netlify.app |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // A bare identifier is a runtime name, not the string 'Function'. Expressions stay loud. | ||
| if (asNode(node)?.type === 'Identifier') return false; | ||
| return literal === undefined; | ||
| } |
There was a problem hiding this comment.
A computed identifier is now considered unable to name Function, even though const key = 'Function' or a loop variable can hold that value at runtime. Both module guards consequently allow paths such as Reflect.get(globalThis, key) to obtain the host Function constructor without the existing routing ceiling, permitting dynamically constructed imports to escape the module loader.
How this was verified: The CJS and ESM guards both delegate computed global keys to this function, which returns false for identifiers and loop bindings regardless of their runtime string value.
| export function trackUnawaitedCliAction(source: string): string { | ||
| if (source.includes(TRACKER) || !source.includes(BARE_CALL)) return source; | ||
| return source.replaceAll( | ||
| BARE_CALL, | ||
| `var __riftyAction = this.runMatchedCommand(); | ||
| if (__riftyAction && typeof __riftyAction.then === "function" && globalThis.${TRACKER}) globalThis.${TRACKER}(__riftyAction);`, | ||
| ); |
There was a problem hiding this comment.
This unrestricted textual replacement runs on every transformed ESM module. An unrelated string, template, or comment containing this.runMatchedCommand(); is therefore rewritten as executable multi-line code, which can make the module fail to compile or change its behavior. The replacement also inserts lines after the source-map line map has been recorded, so later stack locations become incorrect.
| if (lineOffset === 0 && columnOffset === 0) { | ||
| OFFSETS.delete(filename); | ||
| return; | ||
| } | ||
| OFFSETS.set(filename, { line: lineOffset, column: columnOffset }); |
There was a problem hiding this comment.
VM offsets are stored globally by filename rather than with the compiled Script. If two scripts named foo.js use different offsets, the most recently registered offset rewrites both scripts' stack frames. Creating an unoffset script with that filename also deletes the first script's offset, producing incorrect diagnostics whenever filenames are reused.
| function isProcessStdioSink(dest: object): boolean { | ||
| const fd = (dest as { fd?: unknown }).fd; | ||
| return fd === 1 || fd === 2; | ||
| } |
There was a problem hiding this comment.
Skipping end() based only on fd === 1 || fd === 2 misclassifies any custom or file-backed writable carrying one of those descriptor values as process stdio. Piping into such a distinct destination leaves it unterminated, so finish never fires and buffered output may remain unflushed. The exception needs to identify the actual process stdout and stderr sinks rather than every matching fd value.
| const type = typeof message; | ||
| if (type === 'function' || type === 'symbol' || type === 'bigint') { | ||
| throw Object.assign( | ||
| new TypeError( | ||
| 'The "message" argument must be one of type string, object, number, or boolean', | ||
| ), | ||
| { code: 'ERR_INVALID_ARG_TYPE' }, | ||
| ); | ||
| } | ||
| return message; |
There was a problem hiding this comment.
Advanced IPC returns the original object instead of serializing and cloning it. On the same-realm child-process route, the parent and child therefore receive the same reference, so mutations cross the process boundary. Nested unsupported values such as functions are also delivered instead of raising a serialization error; the existing round-trip test only checks types and passes because identity is preserved.
|
|
||
| registerBuiltin('path', () => pathModule); | ||
| registerBuiltin('path/posix', () => pathModule.posix); | ||
| registerBuiltin('path/win32', () => pathModule.win32); |
There was a problem hiding this comment.
Registering node:path/win32 exposes the existing POSIX alias as though it were a supported Node builtin: sep remains /, delimiter remains :, and join('a', 'b') returns a/b. Before this change, the unsupported builtin failed loudly. Returning materially wrong Windows path behavior violates the repository directive that missing features must remain explicit NotImplementedError ceilings.
Context Used: CLAUDE.md (source)
| if (str.startsWith('npm:')) str = str.slice(4); | ||
| const at = str.lastIndexOf('@'); | ||
| if (at <= 0) return { name: str, range: null }; | ||
| return { name: str.slice(0, at), range: str.slice(at + 1) }; | ||
| if (at > 0) return { name: str.slice(0, at), range: str.slice(at + 1) }; | ||
| if (isNpmBareVersionRange(str)) return { name: overriddenName, range: str }; |
There was a problem hiding this comment.
The range heuristic runs after stripping npm:, so a valid alias such as npm:v8 becomes v8 and is classified as a version range of the overridden package rather than as the replacement package name. The resulting name and range directly control installation, causing the installer to resolve the wrong package or fail on a bogus range.
Forked program children still stay alive for json and advanced message listeners, so the workbench owner can exit and vitest's forks pool still runs. Co-authored-by: Cursor <cursoragent@cursor.com>
Final+GREEN is not recorded yet, so the pull request stays a draft until that review. Co-authored-by: Cursor <cursoragent@cursor.com>
The review findings were real: a text rewrite that could corrupt modules, stack offsets that clobbered each other, advanced IPC that shared objects, a pipe end skipped for any fd 1/2 writable, npm:v8 read as a version, path.win32 returning POSIX, and a const 'Function' key slipping the ceiling. Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Summary
npm installaccepts a bare override version ("vite": "8.0.16") and installs that exact vite for vitest 4.1.11.vitest runloadsvitest.config.tsand TypeScript tests, prints the default reporter (1 passed/1 failed), and returns Node's exit code.npm test,--reporter=verbose, and--pool=threadsmatch.docs/public/compat/vitest.md.Test plan
pnpm pr:checktests/e2e/vitest-run.spec.ts(chromium-heavy): clean install, failing run exits 1, fixed run exits 0, threads pool matchesMade with Cursor