Audit and document Vite optimizeDeps declarations - #140
Merged
Conversation
arybakov05
reviewed
Jun 14, 2026
arybakov05
left a comment
arybakov05
left a comment
Contributor
There was a problem hiding this comment.
I'm not very familiar with all the TypeScript AST wizardry, so I had the AI explain the script to me. Together, we noticed some small issues. Apart from that, the script seems to work fine for missing client optimizeDeps entries. What I did notice, however, is that it doesn't pick up missing entries in the ssr.optimizeDeps.
| ssr: { | ||
| optimizeDeps: { | ||
| include: ['i18next-fs-backend/cjs', 'remix-i18next/server'], | ||
| include: ['i18next-fs-backend/cjs', 'isbot', 'remix-i18next/server'], |
Contributor
There was a problem hiding this comment.
I tried running from this feature branch locally, and it seems that Vite is still picking up the i18next-fs-backend/cjs and remix-i18next/server dependencies for the client optimizer.
The fix is to explicitly exclude them in the client optimizeDeps as such:
optimizeDeps: {
exclude: [
'i18next-fs-backend',
'i18next-fs-backend/cjs',
'remix-i18next/server',
],
include: [
...
]
| if (node.text.startsWith(prefix)) { | ||
| declared.add(node.text.slice(prefix.length)); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
This is broad and would also catch dependencies declared in the optimizeDeps.exclude, which would falsely mark excluded dependencies as declared
|
|
||
| if ( | ||
| ts.isExportDeclaration(node) && | ||
| !node.isTypeOnly && |
Contributor
There was a problem hiding this comment.
This only checks isTypeOnly per node, but not per element. Something like export { type Foo } from 'bar' would be a false positive
- Declare typescript as an explicit devDependency of @plone/scripts instead of relying on it being hoisted from the repo root - Fix incorrect @openai attribution in the audit script news fragment - Dedupe the "OK" output line when multiple targets share a config file - Wire pnpm check:vite-optimize-deps into the Code Analysis CI workflow so drift between runtime imports and optimizeDeps declarations is caught automatically
- Exclude i18next-fs-backend and remix-i18next/server from the client
optimizeDeps bundle: Vite's client-side scanner was still picking
them up even though they were only declared under
ssr.optimizeDeps.include
- Scope check-vite-optimize-deps.js's declared-entries scan to actual
`include` arrays instead of any string literal in the config file,
so entries listed under `optimizeDeps.exclude` are no longer
mistaken for satisfied declarations
- Check named re-exports per-element for `isTypeOnly` instead of only
at the export-statement level, so `export { type Foo } from 'pkg'`
is no longer treated as a runtime dependency
- Split expected specifiers into client vs. server-only (based on
`*.server.*` filenames) and diff them against optimizeDeps.include
and ssr.optimizeDeps.include separately, so drift in the SSR-only
list is now actually caught
- Document the client-exclude gotcha in vite-optimize-deps.md
Member
Author
|
@arybakov05 I worked on this a bit more, and created a CI workflow for check the introduction of new deps. Despite the fact that I don't know much either about AST parsing (let the machine take care of it), looks good and I think it's a great addition/fix. @pnicolli what do you think? If you both give me your blessing, I will merge #134 and this one. |
sneridagh
added a commit
that referenced
this pull request
Jul 29, 2026
) * peed up dev startup by pre-bundling external deps in optimizeDeps * speed up dev startup by pre-bundling external deps in optimizeDeps * remove documentation line breaks * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Audit and document Vite optimizeDeps declarations (#140) * Audit and document Vite optimizeDeps declarations * Address review feedback on optimizeDeps audit script - Declare typescript as an explicit devDependency of @plone/scripts instead of relying on it being hoisted from the repo root - Fix incorrect @openai attribution in the audit script news fragment - Dedupe the "OK" output line when multiple targets share a config file - Wire pnpm check:vite-optimize-deps into the Code Analysis CI workflow so drift between runtime imports and optimizeDeps declarations is caught automatically * Update pnpm-lock.yaml for typescript devDependency in @plone/scripts * Address arybakov05 review feedback on PR #140 - Exclude i18next-fs-backend and remix-i18next/server from the client optimizeDeps bundle: Vite's client-side scanner was still picking them up even though they were only declared under ssr.optimizeDeps.include - Scope check-vite-optimize-deps.js's declared-entries scan to actual `include` arrays instead of any string literal in the config file, so entries listed under `optimizeDeps.exclude` are no longer mistaken for satisfied declarations - Check named re-exports per-element for `isTypeOnly` instead of only at the export-statement level, so `export { type Foo } from 'pkg'` is no longer treated as a runtime dependency - Split expected specifiers into client vs. server-only (based on `*.server.*` filenames) and diff them against optimizeDeps.include and ssr.optimizeDeps.include separately, so drift in the SSR-only list is now actually caught - Document the client-exclude gotcha in vite-optimize-deps.md --------- Co-authored-by: Víctor Fernández de Alba <sneridagh@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
Stacked on top of #134.