Skip to content

chore(dashboard): convert vite config to TypeScript - #383

Merged
harshtandiya merged 2 commits into
developfrom
chore/vite-config-ts
Aug 27, 2026
Merged

chore(dashboard): convert vite config to TypeScript#383
harshtandiya merged 2 commits into
developfrom
chore/vite-config-ts

Conversation

@harshtandiya

Copy link
Copy Markdown
Collaborator

What changed

Renames dashboard/vite.config.js to .ts and adds it to tsconfig.json's include, which covered only src/ and tests/ — so the vite config was type-checked by nothing.

The rename alone would be cosmetic; defineConfig already gives autocomplete in a .js file. What makes it worth doing is the second half. frappe-ui/vite ships as plain JavaScript with no declarations at all (no types field, no .d.ts under vite/), so converting forced an ambient declaration. That declaration covers only the options frappe-ui's plugins actually read, which turns the previous PR's bug class into a compile error:

vite.config.ts(21,5): error TS2353: Object literal may only specify known
properties, and 'target' does not exist in type 'BuildConfigOptions'.

That is #382's dead target key, caught at typecheck time. Same for chunkSizeWarningLimit. The option shapes were read off buildConfig.js, frappeProxy.js, lucideIcons.js and index.js rather than guessed.

Not changed: tailwind.config.js and postcss.config.js stay .js and stay unchecked — they would need allowJs or their own conversion, and neither is carrying dead config. __dirname needed no special handling despite "type": "module"; @types/node covers it.

Stacked on #382, which is stacked on #381. Review in that order.

Demo

No visual change — build config only. Applying skip-demo.

Testing

yarn typecheck clean, yarn build passes (1.92s, same output). pre-commit run --files green on all three files. Verified the declaration works by temporarily re-adding target: "es2015" and confirming TS2353, then removing it and confirming clean.

🤖 Generated with Claude Code

@harshtandiyaharshtandiya added the skip-demo Skip adding a video / screenshot in PR description. Passes the failing CI for demo addition label Aug 27, 2026
@greptile-apps

greptile-appsBot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR converts the dashboard Vite configuration to TypeScript and brings it under the dashboard typecheck.

  • Adds an ambient declaration for the untyped frappe-ui/vite module.
  • Includes the Vite configuration and declaration file in tsconfig.json.
  • Preserves the existing Vite runtime configuration while renaming it to vite.config.ts.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

FilenameOverview
dashboard/frappe-ui-vite.d.tsAdds focused typings for the frappe-ui Vite plugin and the option shapes used by the dashboard configuration.
dashboard/tsconfig.jsonExtends typecheck coverage to the Vite configuration and its ambient module declaration.
dashboard/vite.config.tsRenames the existing Vite configuration to TypeScript without changing its effective build behavior.

Reviews (2): Last reviewed commit: "chore(dashboard): trim vite config comme..." | Re-trigger Greptile

Base automatically changed from chore/vite-dead-config to developAugust 27, 2026 11:40
harshtandiyaand others added 2 commits August 27, 2026 11:44
`tsconfig.json` included only `src/` and `tests/`, so the vite config was
checked by nothing. Renaming it and adding it to `include` puts it under
`yarn typecheck`.
The conversion needs `frappe-ui/vite` to have types, which it does not ship —
it is plain JavaScript with no declarations. The ambient declaration added here
covers only the options its plugins actually read, which is what makes this
worth doing: passing a key frappe-ui ignores is now a type error instead of
config that looks applied and is not. Re-adding the `target` key removed in the
previous commit fails typecheck with TS2353.
`tailwind.config.js` and `postcss.config.js` are still unchecked. They would
need `allowJs` or their own conversion, and neither is carrying dead config.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Drops the link-to-docs line and cuts the two optimizeDeps comments to the part
that is not recoverable from the code: why frappe-ui is excluded and why its
nested feather-icons is not.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@harshtandiyaharshtandiya added the backport main backport to main branch label Aug 27, 2026
@harshtandiya
harshtandiya merged commit fa49530 into developAug 27, 2026
11 checks passed
@harshtandiya
harshtandiya deleted the chore/vite-config-ts branch August 27, 2026 11:54
@github-actions

Copy link
Copy Markdown
Contributor

Successfully created backport PR for main:

harshtandiya added a commit that referenced this pull request Aug 27, 2026
chore(dashboard): convert vite config to TypeScript (#383)
* chore(dashboard): convert vite config to TypeScript
`tsconfig.json` included only `src/` and `tests/`, so the vite config was
checked by nothing. Renaming it and adding it to `include` puts it under
`yarn typecheck`.
The conversion needs `frappe-ui/vite` to have types, which it does not ship —
it is plain JavaScript with no declarations. The ambient declaration added here
covers only the options its plugins actually read, which is what makes this
worth doing: passing a key frappe-ui ignores is now a type error instead of
config that looks applied and is not. Re-adding the `target` key removed in the
previous commit fails typecheck with TS2353.
`tailwind.config.js` and `postcss.config.js` are still unchecked. They would
need `allowJs` or their own conversion, and neither is carrying dead config.
* chore(dashboard): trim vite config comments
Drops the link-to-docs line and cuts the two optimizeDeps comments to the part
that is not recoverable from the code: why frappe-ui is excluded and why its
nested feather-icons is not.
---------
(cherry picked from commit fa49530)
Co-authored-by: Harsh Tandiya <harsh.tandiya@gmail.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport mainbackport to main branchskip-demoSkip adding a video / screenshot in PR description. Passes the failing CI for demo addition

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@harshtandiya