chore: avoid missing file extension warning in test/examples - #1775
chore: avoid missing file extension warning in test/examples#1775yamachi4416 wants to merge 4 commits into
Conversation
commit: |
yamachi4416
changed the title
optimizeDeps.include
danielroe
left a comment
danielroe
left a comment
There was a problem hiding this comment.
do we really need a test fixture for this?
📝 WalkthroughWalkthroughAdded the Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
examples/use-cjs/package.json (1)
18-18: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPin
@nuxt/test-utilsto the tested release.
examples/use-cjs/vitest.config.tsusesdefineVitestProjectfor both projects. This manifest pins Nuxt and Vitest but uses the movinglatesttag for@nuxt/test-utils. The Nuxt changelog records browser setup support in@nuxt/test-utils4.1.0, so pin the version tested with Nuxt 4.5.2 and Vitest 4.1.10. (nuxt.com)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/use-cjs/package.json` at line 18, Update the `@nuxt/test-utils` dependency in the examples/use-cjs package manifest from the moving latest tag to the tested 4.1.0 release, keeping the existing Nuxt and Vitest versions unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/use-cjs/libs/cjs-wrapper/index.d.ts`:
- Line 1: Update the declaration in index.d.ts to represent the default export
provided by index.mjs, while preserving the existing hello named export if
required. Model the default import’s actual runtime shape rather than reusing
the cjs-pure CommonJS object declaration.
In `@examples/use-cjs/nuxt.config.ts`:
- Around line 6-11: Update the optimizeDeps configuration in nuxt.config.ts to
restore noDiscovery: true alongside the existing include list, preserving
optimization of only the two listed packages.
---
Nitpick comments:
In `@examples/use-cjs/package.json`:
- Line 18: Update the `@nuxt/test-utils` dependency in the examples/use-cjs
package manifest from the moving latest tag to the tested 4.1.0 release, keeping
the existing Nuxt and Vitest versions unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 02065eb0-e22c-4f4d-af7c-d2d27359179a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (28)
.gitignoreexamples/app-vitest-workspace/vitest.config.tsexamples/use-cjs/app/app.vueexamples/use-cjs/app/components/CjsPureDefault.vueexamples/use-cjs/app/components/CjsPureNamespace.vueexamples/use-cjs/app/components/CjsWrapperDefault.vueexamples/use-cjs/app/components/CjsWrapperNamespace.vueexamples/use-cjs/app/composables/useCjsPureDefault.tsexamples/use-cjs/app/composables/useCjsPureNamespace.tsexamples/use-cjs/app/composables/useCjsWrapperDefault.tsexamples/use-cjs/app/composables/useCjsWrapperNamespace.tsexamples/use-cjs/libs/cjs-pure/index.cjsexamples/use-cjs/libs/cjs-pure/index.d.tsexamples/use-cjs/libs/cjs-pure/package.jsonexamples/use-cjs/libs/cjs-wrapper/index.cjsexamples/use-cjs/libs/cjs-wrapper/index.d.tsexamples/use-cjs/libs/cjs-wrapper/index.mjsexamples/use-cjs/libs/cjs-wrapper/package.jsonexamples/use-cjs/nuxt.config.tsexamples/use-cjs/package.jsonexamples/use-cjs/test/nuxt/app-mount.spec.tsexamples/use-cjs/test/nuxt/cjs-components.spec.tsexamples/use-cjs/test/nuxt/cjs-composables-auto.spec.tsexamples/use-cjs/test/nuxt/cjs-composables-manual.spec.tsexamples/use-cjs/test/nuxt/cjs-import-default.spec.tsexamples/use-cjs/test/nuxt/cjs-import-namespace.spec.tsexamples/use-cjs/tsconfig.jsonexamples/use-cjs/vitest.config.ts
| @@ -0,0 +1 @@ | |||
| export declare function hello(): string | |||
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Candidate files =="
fd -a 'index\.(ts|js|mjs|d\.ts|tsconfig.*|package\.json|.*\.vue)$' examples/use-cjs | sort
echo
echo "== cjs-wrapper files =="
for f in \
examples/use-cjs/libs/cjs-wrapper/index.d.ts \
examples/use-cjs/libs/cjs-wrapper/index.mjs \
examples/use-cjs/libs/cjs-wrapper/package.json \
examples/use-cjs/libs/cjs-pure/index.d.ts \
examples/use-cjs/libs/cjs-pure/index.mjs \
examples/use-cjs/libs/cjs-pure/package.json \
examples/use-cjs/app/components/CjsWrapperDefault.vue
do
echo "--- $f"
cat -n "$f"
done
echo
echo "== TS config / package metadata =="
for f in tsconfig*.json package.json example*/**/tsconfig*.json example*/**/package.json 2>/dev/null; do
[ -f "$f" ] && echo "--- $f" && cat "$f"
done
echo
echo "== Search usage/import declarations =="
rg -n "from ['\"]@.*cjs|import \{|import .* \{" examples/use-cjs || trueRepository: nuxt/test-utils
Length of output: 1286
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== use-cjs tracked files =="
git ls-files examples/use-cjs | sort
echo
echo "== all occurrences mentioning cjs wrapper/pure/default import =="
rg -n "hello|CjsWrapperDefault|cjs-wrapper|cjs-pure|default|types|import:" examples/use-cjs || true
echo
echo "== package metadata in repository =="
cat -n package.json
echo
echo "== package scripts and type-check configuration =="
python3 - <<'PY'
import json, pathlib
p = pathlib.Path('package.json')
if p.exists():
data=json.loads(p.read_text())
print(json.dumps(data.get('scripts',{}), indent=2))
print({k:v for k,v in data.items() if k in ('type','dependencies','devDependencies')})
PY
echo
echo "== cjs-wrapper source =="
for f in \
examples/use-cjs/libs/cjs-wrapper/index.d.ts \
examples/use-cjs/libs/cjs-wrapper/index.mjs \
examples/use-cjs/libs/cjs-wrapper/index.cjs \
examples/use-cjs/libs/cjs-wrapper/package.json \
examples/use-cjs/libs/cjs-pure/index.d.ts
do
[ -f "$f" ] && { echo "--- $f"; cat -n "$f"; }
doneRepository: nuxt/test-utils
Length of output: 16870
Fix the cjs-wrapper declaration for the default ESM import.
examples/use-cjs/libs/cjs-wrapper/index.mjs exports default, but index.d.ts only declares hello. TypeScript cannot type-check CjsWrapperDefault.vue with the current declaration, because the package’s runtime import contract is unrepresented.
Add a default export declaration or separate ESM/CommonJS declarations; cjs-pure/index.d.ts already models the CJS object shape and is not directly a valid ESM declaration here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/use-cjs/libs/cjs-wrapper/index.d.ts` at line 1, Update the
declaration in index.d.ts to represent the default export provided by index.mjs,
while preserving the existing hello named export if required. Model the default
import’s actual runtime shape rather than reusing the cjs-pure CommonJS object
declaration.
| optimizeDeps: { | ||
| include: [ | ||
| 'example-use-cjs-cjs-pure', | ||
| 'example-use-cjs-cjs-wrapper', | ||
| ], | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repository files matching relevant names:\n'
rg -n "optimizeDeps|noDiscovery|example-use-cjs|examples/use-cjs" -S . || true
printf '\nRelevant config snippet:\n'
if [ -f examples/use-cjs/nuxt.config.ts ]; then
nl -ba examples/use-cjs/nuxt.config.ts | sed -n '1,35p'
fi
printf '\nDiff/stat context:\n'
git diff --stat || true
git diff -- examples/use-cjs/nuxt.config.ts || true
printf '\nPackage manifests:\n'
if [ -f examples/use-cjs/package.json ]; then
cat examples/use-cjs/package.json
fiRepository: nuxt/test-utils
Length of output: 3123
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'examples/use-cjs/nuxt.config.ts lines 1-25:\n'
awk '{printf "%5d\t%s\n", NR, $0}' examples/use-cjs/nuxt.config.ts | sed -n '1,25p'
printf '\nexamples/use-cjs/package.json:\n'
cat examples/use-cjs/package.json
printf '\nsrc/config.ts optimizeDeps context:\n'
awk '{printf "%5d\t%s\n", NR, $0}' src/config.ts | sed -n '180,200p'
printf '\nDiff for examples/use-cjs/nuxt.config.ts:\n'
git diff -- examples/use-cjs/nuxt.config.ts || true
printf '\nSearch for noDiscovery in code/tests:\n'
rg -n "noDiscovery|optimizeDeps" -S src examples || trueRepository: nuxt/test-utils
Length of output: 2377
Restore optimizeDeps.noDiscovery in examples/use-cjs/nuxt.config.ts.
The example should exercise the no-discovery configuration while limiting optimization to the listed packages; keep include and add noDiscovery: true inside vite.optimizeDeps.
Proposed fix
optimizeDeps: {
+ noDiscovery: true,
include: [📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| optimizeDeps: { | |
| include: [ | |
| 'example-use-cjs-cjs-pure', | |
| 'example-use-cjs-cjs-wrapper', | |
| ], | |
| }, | |
| optimizeDeps: { | |
| noDiscovery: true, | |
| include: [ | |
| 'example-use-cjs-cjs-pure', | |
| 'example-use-cjs-cjs-wrapper', | |
| ], | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/use-cjs/nuxt.config.ts` around lines 6 - 11, Update the optimizeDeps
configuration in nuxt.config.ts to restore noDiscovery: true alongside the
existing include list, preserving optimization of only the two listed packages.
|
@danielroe |
yamachi4416
changed the title
optimizeDeps.include 2 participants
🔗 Linked issue
📚 Description
Add test for cjs module adding tovite.optimizeDeps.includein nuxt.config.ts, and post a comment to a solution to the issue: #1388avoid missing file extension warning
ignore vitest test results in .gitignore