Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the project configuration, guidelines, and dependencies, including transitioning from .tool-versions to mise.toml, switching Bun's linker to isolated in bunfig.toml, and adding a comprehensive E2E test fixture. Feedback on the changes suggests restoring the exclusion of e2e/fixture from the root tsconfig.json to prevent type-checking errors and eliminate redundancy, as well as reverting the empty typescript peer dependency range in package.json back to "*" to ensure compatibility.
| "exclude": ["e2e/fixture/test/fixtures", "test/fixtures"], | ||
| "extends": "@tsconfig/bun/tsconfig.json", | ||
| "include": ["*.config.ts", "e2e/**/*", "scripts/**/*", "src/**/*", "test/**/*", "types/**/*"] | ||
| "include": [ | ||
| "*.config.ts", | ||
| "e2e/**/*", | ||
| "e2e/fixture/*.config.ts", | ||
| "e2e/fixture/scripts/**/*", | ||
| "e2e/fixture/src/**/*", | ||
| "e2e/fixture/test/**/*", | ||
| "scripts/**/*", | ||
| "src/**/*", | ||
| "test/**/*", | ||
| "types/**/*" | ||
| ] |
There was a problem hiding this comment.
Including e2e/fixture files in the root tsconfig.json compilation will cause type-checking errors (e.g., during bun wb typecheck) because the root configuration lacks the specific path mappings (like next/link shims) and dependencies (like vinext) defined in the fixture's own tsconfig.json and package.json.
Additionally, explicitly listing subdirectories of e2e/fixture in include is redundant since e2e/**/* is already included, violating the repository style guide rule to eliminate redundancy.
We should restore the exclusion of e2e/fixture from the root compilation and let it be type-checked independently via its own configuration.
"exclude": ["e2e/fixture", "test/fixtures"],
"extends": "@tsconfig/bun/tsconfig.json",
"include": [
"*.config.ts",
"e2e/**/*",
"scripts/**/*",
"src/**/*",
"test/**/*",
"types/**/*"
]References
- Simplify code as much as possible to eliminate redundancy. Including specific subdirectories of
e2e/fixturewhene2e/**/*is already included is redundant. (link)
| "peerDependencies": { | ||
| "react": ">=18", | ||
| "typescript": "*" | ||
| "typescript": "" |
There was a problem hiding this comment.
Using an empty string "" as a version range for typescript in peerDependencies is highly non-standard and can cause warnings or resolution failures in strict package managers. It should be restored to "*" to indicate compatibility with any version.
| "typescript": "" | |
| "typescript": "*" |
Update to the latest wbfy (wb 16) and fix two issues the isolated-linker migration exposed: - Point the fixture tsconfig's next/* shim paths at the fixture-local node_modules, where the isolated linker now places vinext (previously hoisted to the repo root). - The root tsconfig now excludes e2e/fixture/app so the framework fixture's app sources are type-checked only by their own tsconfig (which shims next/* via paths), not by the root project (fixed in wbfy itself). Co-authored-by: WillBooster (Claude Code) <agent@willbooster.com>
Applies the latest wbfy (wb 16) to this repository.
Notable changes
mise.toml, renamedrenovate.json→renovate.jsonc, and refreshed shared config/doc files.typescriptpeer dependency.Fixes for isolated-linker type checking
The isolated-linker migration exposed two type-checking breakages, now fixed:
next/*shims are now resolved from the fixture-localnode_modules, where the isolated linker placesvinext(previously hoisted to the repo root).tsconfig.jsonnow excludese2e/fixture/app, so the fixture's app sources are type-checked only by their own tsconfig (which shimsnext/*viapaths), not by the root project. This was fixed generally in wbfy itself: fix(wbfy): exclude framework workspace app dirs from the root tsconfig shared#1062.bun verifypasses.