chore(core): align tsconfig moduleResolution with bundler - #3475
Conversation
Next.js treats moduleResolution as a required compiler option and, for TypeScript >= 5 with module=esnext, only accepts bundler/node16/nodenext. With "node" it rewrote every store's generated .faststore/tsconfig.json on each dev/build and logged "reconfigured your tsconfig.json". Every other package in the monorepo already uses "bundler"; aligning core removes the drift between the committed config and what Next actually type-checks with.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (1)
Included review availability: 3 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. WalkthroughThe core package changes TypeScript module resolution from ChangesCore TypeScript configuration
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This localized configuration change aligns generated store settings with the Next.js build behavior and prevents repeated tsconfig rewrites and warnings; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
0 New Issues
0 Fixed Issues
0 Accepted Issues
No data about coverage (37.40% Estimated after merge)
|
Validated this against a real store (Yarn 1 classic, Next.js 16.2.6, TypeScript 5.9.3, Confirmed the fix works as described:
Unrelated note, but worth flagging for other Yarn Classic (v1) testers: installing this build initially failed at runtime with: This isn't caused by this PR's change — it's inherent to the v4 base This is the same class of issue the official v4 upgrade guide already documents for "resolutions": {
"signal-exit": "^4.1.0"
}fixed it for me. Might be worth adding to that same doc/guide alongside the Also re-tested against the combined #3478 integration build ( |
) ## What's the purpose of this pull request? A store cloned under a directory whose name contains a space (e.g. `/Users/me/My Store/store`, `C:\Users\John Doe\...`) cannot run `faststore build` / `faststore dev` / `faststore generate`: ``` [STARTED] Load GraphQL documents [FAILED] Failed to load documents from /Users/me/My Store/store/.faststore/src/**/*.{ts,tsx},!/Users/me/My Store/store/.faststore/@generated/: [FAILED] Failed to parse the GraphQL document. Syntax Error: Unexpected character: "/". ``` This had been assumed fixed by #3454 — it is not: #3454 fixed the `next` binary resolution, this is a different failure in the GraphQL codegen step. Reproduced on `4.6.0` and on `dev` (`4.7.1-dev.2`) with a real Yarn 1 workspaces store cloned under a path with a space. Root cause: `generate-types.ts` passed the codegen `documents` glob and the `generates` output dir as **absolute paths**. `@graphql-tools/load` (`isDocumentString`) treats any pointer that contains whitespace and is not parseable SDL as a broken inline document and throws instead of falling through to the glob loader. The `@graphql-codegen/client-preset` also appends `!<outputDir>` to the documents, so the output dir hit the same check. ## How it works? - New `getCodegenPointers(root, cwd = process.cwd())` builds the `documents` glob, the `schema` path and the output dir **relative to the directory codegen resolves pointers from**, so the parent directories (where the space lives) are never part of the pointer: - `.faststore/src/**/*.{ts,tsx}` / `.faststore/@generated/schema.graphql` / `.faststore/@generated/` - Glob-special characters in the relative segments are still escaped (`globby.convertPathToPattern`, same approach as #3454 for the typeDefs glob); the schema and output dir are plain paths, not globs, so they are not escaped. - Symlinks are resolved on both sides (`/var` → `/private/var` on macOS) so the relative path does not climb back through the parents. - `genTsTypes` now awaits the codegen run (it was fire-and-forget). - CLI `vitest.config.ts`: alias `graphql` to its CJS build. The code under test and the externalized `@graphql-tools/*` otherwise load two `graphql` instances and fail `instanceof` checks ("Duplicate graphql modules"), which made an end-to-end test of the generator impossible. No behaviour change for paths without spaces: the same files are matched and written, only the pointer form changes. ## How to test it? - `cd packages/cli && pnpm test` — 177 tests. New in `generate-types.test.ts`: - `getCodegenPointers` never embeds the absolute project path, targets `.faststore` when present, escapes glob-special characters only in the glob. - `genTsTypes` end-to-end from a store under `<tmp>/faststore gen (x86) XXXX/store`: merges a custom `thirdParty` typeDef and generates `graphql.ts` containing the query type. - Manual: clone any store into a directory with a space, install this CLI build and run `yarn build`. On `dev` it fails at "Load GraphQL documents"; with this branch the generate step completes. ### Starters Deploy Preview Pending CodeSandbox / pkg.pr.new build for this PR. ## References - #3454 (`next` binary resolution — different issue, same symptom family) - Companion PRs (independent): #3474, #3475, #3477 Made with [Cursor](https://cursor.com) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved CLI TypeScript type generation for projects located in paths containing spaces, special characters, or symlinks. * Ensured type generation completes reliably before the CLI proceeds. * **Tests** * Expanded automated coverage for schema discovery, path handling, and generated TypeScript output. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
What's the purpose of this pull request?
On v4.6.0, Next.js rewrites the generated
.faststore/tsconfig.jsonon everydev/buildand logsWe detected TypeScript in your project and reconfigured your tsconfig.json file.@faststore/clicopiespackages/core/tsconfig.jsonalmost verbatim into each store's.faststore/tsconfig.json, and that file still had"moduleResolution": "node". Next 16 (next/dist/lib/typescript/writeConfigurationDefaults.js) treatsmoduleResolutionas a required option: for TypeScript >= 5 withmodule: esnextit only acceptsbundler | node16 | nodenext, so"node"is rewritten to"bundler"before Next runs its own type-check — every time, in every store.How it works?
One-line change:
packages/core/tsconfig.jsonmoduleResolutiongoes from"node"to"bundler", matching the roottsconfig.jsonand every other package in the monorepo (api,sdk,ui,components,diagnostics).This is not a behaviour change for stores: Next already forces
"bundler"on the file it type-checks with, sonext buildhas been running under bundler resolution all along. The change only (a) removes the drift between the committed config and what Next actually uses, so the "reconfigured your tsconfig.json" message stops appearing, and (b) makes editor / standalonetscdiagnostics match the real build.How to test it?
cd packages/core && pnpm exec tsc --noEmit -p tsconfig.jsonwith and without--moduleResolution node: the outputs are byte-identical (same 122 pre-existing errors, all undertest/**, zero undersrc/).pnpm lintandcd packages/cli && pnpm test(173 tests, including the.faststoretsconfig copy tests) pass.yarn dev/yarn buildand confirm thereconfigured your tsconfig.jsonmessage is gone and.faststore/tsconfig.jsonis left untouched by Next. Verified in a Yarn 1 workspaces monorepo: the generated.faststore/tsconfig.jsonalready carries"moduleResolution": "bundler"and Next no longer reports a reconfiguration.Starters Deploy Preview
Pending CodeSandbox build for this PR.
References
next/dist/lib/typescript/writeConfigurationDefaults.js(moduleResolutionis in the required set;preferBundlerResolution= TS >= 5.0 && module not commonjs/amd)Made with Cursor
Summary by CodeRabbit