fix(ci): tsc build for package/core is idempodent #12112
Conversation
Summary of ChangesHello @mattKorwel, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where the TypeScript build process for the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request reverts a change in packages/core/tsconfig.json to fix a CI build idempotency issue by removing src/**/*.d.ts from the include array. While this likely resolves the immediate problem, I have added a review comment highlighting the potential risk of this change unintentionally excluding necessary ambient type declarations. I've also suggested a more robust, long-term architectural solution to handle such declaration files.
| "types": ["node", "vitest/globals"] | ||
| }, | ||
| "include": ["index.ts", "src/**/*.ts", "src/**/*.d.ts", "src/**/*.json"], | ||
| "include": ["index.ts", "src/**/*.ts", "src/**/*.json"], |
There was a problem hiding this comment.
Removing src/**/*.d.ts from include may cause build failures if any manually-created ambient declaration files exist within the src directory. TypeScript will no longer automatically include these files, which can break type resolution for global augmentations or modules that are not explicitly imported (e.g., for assets like CSS/images or untyped dependencies), resulting in errors like TS2307: Cannot find module '...'.
While this change likely fixes the idempotency issue, a more robust long-term solution is to isolate necessary ambient declarations. Consider moving any such .d.ts files to a dedicated top-level types directory and including it via the typeRoots compiler option. This makes their global nature explicit and avoids potential conflicts with source files.
There was a problem hiding this comment.
@cornmander this is likely relevant to the changes you were tryign to make originally in #11157
There was a problem hiding this comment.
I think it's fine, the tsignore at
makes this work even w/o the type file.
|
Size Change: -2 B (0%) Total Size: 20.2 MB ℹ️ View Unchanged
|
…dent (google-gemini#12112) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
2 participants
reverting changes to tsconfig.json