Skip to content

Conversation

@fi3ework
Copy link
Member

@fi3ework fi3ework commented Oct 28, 2025

Summary

warn user when they're using some package but not declared in dependencies or devDependencies in bundleless mode, since nothing will be bundled then.

Related Links

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

Copilot AI review requested due to automatic review settings October 28, 2025 03:16
Copy link
Contributor

Copilot AI left a 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);
Copy link

Copilot AI Oct 28, 2025

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.

Suggested change
console.log(warnLogs);

Copilot uses AI. Check for mistakes.
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".')
Copy link

Copilot AI Oct 28, 2025

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.

Copilot uses AI. Check for mistakes.
return;
}

warnDevDependency(request);
Copy link

Copilot AI Oct 28, 2025

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.

Suggested change
warnDevDependency(request);
warnDevDependency(request, issuer);

Copilot uses AI. Check for mistakes.
@netlify
Copy link

netlify bot commented Oct 28, 2025

Deploy Preview for rslib ready!

Name Link
🔨 Latest commit da47931
🔍 Latest deploy log https://app.netlify.com/projects/rslib/deploys/690036d5ccb4ab00088d905f
😎 Deploy Preview https://deploy-preview-1295--rslib.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@fi3ework fi3ework marked this pull request as draft October 28, 2025 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants