Audit and document Vite optimizeDeps declarations#140
Conversation
arybakov05
left a comment
arybakov05
left a comment
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'], |
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)); | ||
| } | ||
| } |
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 && |
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
|
@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. |
2 participants
Summary
Validation
Stacked on top of #134.