Skip to content

Conversation

@9aoy
Copy link
Contributor

@9aoy 9aoy commented Sep 29, 2025

Summary

Update node builtin modules, missing _http_common,_http_agent, _http_client, etc.

https://neos21.github.io/frontend-sandboxes/node-js-builtin-modules/index.html

Related Links

#602

Checklist

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

Copilot AI review requested due to automatic review settings September 29, 2025 06:30
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

Updates Node.js builtin module detection to fix missing private modules like _http_common, _http_agent, and _http_client by leveraging Node's native isBuiltin() function instead of maintaining a manual list.

  • Replaces manual NODE_BUILTINS array with Node's native isBuiltin() for comprehensive builtin detection
  • Renames remaining manual list to ADDITIONAL_NODE_BUILTINS for non-standard builtins like Yarn PnP
  • Adds test coverage for Node builtin module loading functionality

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
packages/core/src/utils/helper.ts Removes manual NODE_BUILTINS list, renames to ADDITIONAL_NODE_BUILTINS
packages/core/src/core/plugins/external.ts Adds isBuiltin import and updates logic to use native detection
e2e/dom/index.test.ts Adds test case for Node builtin module loading
e2e/dom/fixtures/test/node.test.ts Creates test fixture to verify _http_common module loading

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@@ -0,0 +1,9 @@
// @ts-expect-error
Copy link

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The @ts-expect-error comment lacks explanation. Consider adding a brief comment explaining why TypeScript types are not available for this private Node module, e.g., // @ts-expect-error - Private Node module _http_common lacks TypeScript definitions

Suggested change
// @ts-expect-error
// @ts-expect-error - Private Node module _http_common lacks TypeScript definitions

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

netlify bot commented Sep 29, 2025

Deploy Preview for rstest-dev ready!

Name Link
🔨 Latest commit 9fff7e3
🔍 Latest deploy log https://app.netlify.com/projects/rstest-dev/deploys/68da2a650cc3ed00084ed536
😎 Deploy Preview https://deploy-preview-604--rstest-dev.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.

@9aoy 9aoy merged commit 30db6e8 into main Sep 29, 2025
22 of 26 checks passed
@9aoy 9aoy deleted the node-builtin-modules branch September 29, 2025 07:00
@9aoy 9aoy mentioned this pull request Sep 30, 2025
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.

1 participant