-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Replace tsup with esbuild and precompile deterministic Playwright suite #1095
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
base: main
Are you sure you want to change the base?
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Summary
This PR successfully migrates from tsup to esbuild for build tooling, addressing issue #263. The migration introduces dedicated build scripts for library, CLI, and end-to-end test compilation, replacing the previous tsup-based approach.
Key changes:
- Removed tsup dependency and replaced with esbuild (already present)
- Created modular build scripts:
build-lib.mjs
,build-cli.mjs
,build-e2e.mjs
,run-playwright.mjs
- Updated package.json scripts to use new build scripts instead of tsup commands
- Modified TypeScript config to only emit declaration files since esbuild handles JS compilation
- Added sophisticated e2e test precompilation with import rewriting and import.meta compatibility
The new build system addresses the "no more __name
explosions" mentioned in the PR description by adding explicit __name
helpers in the esbuild banner. The e2e build script precompiles Playwright tests and rewrites Stagehand imports to use the local build, eliminating runtime compilation issues.
Confidence Score: 4/5
- This PR is safe to merge with minimal risk
- Score reflects well-structured migration with proper error handling and compatibility measures, but one minor hardcoded string replacement that could be more robust
- Pay close attention to build-e2e.mjs for the string replacement logic
Important Files Changed
File Analysis
Filename | Score | Overview |
---|---|---|
package.json | 4/5 | Replaced tsup dependency with esbuild, updated build scripts to use new build scripts instead of tsup commands |
scripts/build-lib.mjs | 5/5 | Clean esbuild script for library build with proper external dependency handling and __name helper for node compatibility |
scripts/build-cli.mjs | 4/5 | Comprehensive CLI build script with proper executable permissions, config copying, and npm linking with error handling |
scripts/build-e2e.mjs | 4/5 | Complex e2e build script that precompiles Playwright tests, rewrites imports, and handles import.meta compatibility |
scripts/run-playwright.mjs | 5/5 | Simple pass-through script that runs precompiled Playwright tests, avoiding tsx/ts-node timing issues |
tsconfig.build.json | 5/5 | Updated TypeScript config to only emit declaration files, since esbuild now handles JS compilation |
pnpm-lock.yaml | 5/5 | Removed tsup and its dependencies from lockfile, esbuild already present as dependency |
Sequence Diagram
sequenceDiagram
participant Developer
participant npm as npm/pnpm
participant lib as build-lib.mjs
participant cli as build-cli.mjs
participant e2e as build-e2e.mjs
participant tsc as TypeScript
participant esbuild as esbuild
participant dist as dist/
Developer->>npm: pnpm run build
npm->>npm: lint & gen-version & build-dom-scripts
npm->>lib: node scripts/build-lib.mjs
lib->>esbuild: bundle lib/index.ts
esbuild->>dist: output dist/index.js (ESM)
npm->>tsc: build-types (tsc --project tsconfig.build.json)
tsc->>dist: emit .d.ts files only
Developer->>cli: node scripts/build-cli.mjs
cli->>esbuild: bundle evals/cli.ts
esbuild->>dist: output dist/evals/cli.js (CJS)
cli->>cli: copy config & set permissions
cli->>npm: npm link (if enabled)
Developer->>e2e: node scripts/build-e2e.mjs
e2e->>e2e: collect .ts entry points
e2e->>esbuild: bundle all deterministic tests
esbuild->>dist: output to dist/playwright/
e2e->>e2e: rewrite stagehand imports
e2e->>e2e: patch import.meta compatibility
6 files reviewed, 1 comment
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Hey, i think u can merge this @seanmcguire12 |
Swapped out tsup for esbuild so the Stagehand build stays fast and maintained, and taught Playwright to run against precompiled JS instead of scrambling at runtime. We now bundle the library and CLI with tiny esbuild scripts, compile the deterministic suite ahead of time, rewrite its imports, and keep the helper that Playwright was missing. The net effect: pnpm run build and the local e2e suite run clean again, no more __name explosions, and the build pipeline feels a lot healthier.
covers #263