-
-
Notifications
You must be signed in to change notification settings - Fork 54
feat: warn used dep in devDependencies under bundeless mode #1295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a warning system to detect when bundleless builds externalize dependencies that are declared in devDependencies instead of dependencies or peerDependencies. Since bundleless mode doesn't include devDependencies in the output, this helps developers identify potential runtime issues.
Key Changes:
- Added warning logic to detect and report devDependencies usage in bundleless external resolution
- Integration test to verify the warning is properly triggered for both JS and CSS devDependencies
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/config.ts | Implements devDependency detection and warning logic in composeBundlelessExternalConfig |
| tests/integration/externals/index.test.ts | Adds test case to verify warning appears for devDependencies |
| tests/integration/externals/dev-dependency-warning/src/index.ts | Test fixture that imports devDependencies |
| tests/integration/externals/dev-dependency-warning/rslib.config.ts | Test fixture configuration for bundleless mode |
| tests/integration/externals/dev-dependency-warning/package.json | Test fixture package.json with devDependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| restore(); | ||
| const warnLogs = logs.map((log) => stripAnsi(String(log))); | ||
| console.log(warnLogs); |
Copilot
AI
Oct 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove debug console.log statement before merging to production. This appears to be leftover debugging code.
| console.log(warnLogs); |
| console.log(warnLogs); | ||
| const jsMatchingLog = warnLogs.filter( | ||
| (log) => | ||
| log.includes('The externalized request "left-pad/lib" from index.ts is declared in "devDependencies" in package.json. Bundleless mode does not include devDependencies in the output, consider moving it to "dependencies" or "peerDependencies".') |
Copilot
AI
Oct 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extract the repeated warning message text into a constant to avoid duplication and make future updates easier. The same message is duplicated on line 132 with only the package name difference.
packages/core/src/config.ts
Outdated
| return; | ||
| } | ||
|
|
||
| warnDevDependency(request); |
Copilot
AI
Oct 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The second call to warnDevDependency is missing the issuer parameter, which means the warning check at line 1438 will always return early. This call should include the issuer to properly validate and display warnings.
| warnDevDependency(request); | |
| warnDevDependency(request, issuer); |
✅ Deploy Preview for rslib ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary
warn user when they're using some package but not declared in
dependenciesordevDependenciesin bundleless mode, since nothing will be bundled then.Related Links
Checklist