Skip to content

Conversation

@jhaynie
Copy link
Member

@jhaynie jhaynie commented Oct 3, 2025

Summary by CodeRabbit

  • New Features

    • Automatically detects native Node modules during bundling and installs required native dependencies into the output.
    • Dynamically builds externals from your project configuration instead of using a fixed list.
  • Improvements

    • Produces a sanitized package.json in the bundle (removes nonessential fields) when native installs are needed.
    • Production-aware installs (platform/arch set, dev dependencies omitted) for more reliable builds.
    • Enhanced logging and error handling for the bundling and native-install flow.
  • Behavior

    • No change for projects without native modules.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 3, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Adds dynamic externals aggregation, detects native Node.js modules in node_modules, conditionally sanitizes and writes a minimal package.json to the output, and runs npm install for native dependencies during bundling. Externals for esbuild now derive from configured/common sets plus package.json. Original path preserved when no native modules are found.

Changes

Cohort / File(s) Summary
Bundler: dynamic externals and native install flow
internal/bundler/bundler.go
Imports slice utility; introduces commonExternals/commonExternalsAutoInstalled; builds externals list from defaults plus package.json; scans node_modules for native modules; assembles nativeInstalls (including auto-installed deps); when natives present, writes sanitized package.json (removing dependencies/devDependencies/externals/scripts/keywords/files) to output and runs npm install (prod-aware: platform/arch, omit dev); otherwise preserves prior behavior; adds logging/error handling around new flow.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev
  participant Bundler
  participant FS as File System
  participant Esbuild
  participant NPM as npm

  Dev->>Bundler: bundle(projectDir, outDir)
  Bundler->>FS: read package.json
  Bundler->>FS: scan node_modules for native modules
  alt Native modules found
    Bundler->>Bundler: build externals = common + pkg.externals
    Bundler->>Bundler: collect nativeInstalls (+ auto-installed deps)
    Bundler->>FS: write sanitized package.json to outDir
    note right of Bundler: Remove dependencies, devDependencies, externals, scripts, keywords, files
    Bundler->>NPM: install nativeInstalls (prod flags if applicable)
    NPM-->>Bundler: install result
    Bundler->>Esbuild: build with externals
    Esbuild-->>Bundler: artifacts
  else No native modules
    Bundler->>Bundler: build externals = common + pkg.externals
    Bundler->>Esbuild: build with externals
    Esbuild-->>Bundler: artifacts
  end
  Bundler-->>Dev: bundle result (output in outDir)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I thump my paws—new paths unfurl,
Externals gathered, flags a-whirl.
I sniff for natives in the hay,
Then trim the package, hop away.
npm hums, the builds align—
A carrot-commit: concise, divine. 🥕✨

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch native-externals

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7fbf40e and 99ff3f0.

📒 Files selected for processing (1)
  • internal/bundler/bundler.go (5 hunks)

Comment @coderabbitai help to get the list of available commands and usage tips.

@jhaynie jhaynie marked this pull request as ready for review October 9, 2025 21:25
@jhaynie jhaynie merged commit 199fa7a into main Oct 9, 2025
14 checks passed
@jhaynie jhaynie deleted the native-externals branch October 9, 2025 21:25
@devin-ai-integration devin-ai-integration bot mentioned this pull request Oct 10, 2025
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants