Skip to content

build: bundling configs for .node + proper import of setapp package #217

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.

Sign up for GitHub

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

Open
nsrCodes wants to merge 4 commits into
base: master
Choose a base branch
from
Open

build: bundling configs for .node + proper import of setapp package #217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eaf00bf
draft: bundling configs for .node setapp package
nsrCodes Oct 29, 2025
4858c06
fix: initialize the framework in the right way
nsrCodes Nov 10, 2025
0e7260d
Merge branch 'master' into draft-setapp-wrapper-support
nsrCodes Nov 10, 2025
cf405f7
Merge branch 'master' into draft-setapp-wrapper-support
nsrCodes Nov 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .erb/configs/webpack.config.base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ export default {
loader: "ts-loader",
},
},
{
test: /\.node$/,
loader: "node-loader",
},
],
},

Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@
"jest": "^27.2.5",
"lint-staged": "^11.2.3",
"mini-css-extract-plugin": "^2.4.2",
"node-loader": "^2.1.0",
"prettier": "^2.4.1",
"react-refresh": "^0.14.0",
"rimraf": "^3.0.2",
Expand Down Expand Up @@ -343,4 +344,4 @@
"pre-commit": "lint-staged"
}
}
}
}
4 changes: 2 additions & 2 deletions release/app/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"ws": "^8.16.0"
},
"optionalDependencies": {
"@setapp/framework-wrapper": "^4.3.3"
"@setapp/framework-wrapper": "^4.3.4"
},
"devDependencies": {
"electron": "^27.3.8",
Expand All @@ -34,4 +34,4 @@
"bufferutil": "4.0.3",
"utf-8-validate": "5.0.6"
}
}
}
16 changes: 8 additions & 8 deletions src/main/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ import logger from "../utils/logger";
import { setupIPCForwardingToWebApp } from "./actions/setupIPCForwarding";
import { saveCookies } from "./actions/cookiesHelpers";

if (process.env.IS_SETAPP_BUILD === "true") {
log.log("[SETAPP] build identified")
const setappFramework = require("@setapp/framework-wrapper");
setappFramework.SetappManager.shared.reportUsageEvent(setappFramework.SETAPP_USAGE_EVENT.USER_INTERACTION);
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Major: Reconsider USER_INTERACTION event timing.

Reporting a USER_INTERACTION usage event immediately on app startup (before any actual user interaction) seems semantically incorrect. This event type typically signifies actual user engagement with the application. Consider either:

  1. Reporting a different event type more appropriate for app initialization (e.g., APP_LAUNCH or similar)
  2. Deferring this event until an actual user interaction occurs
🤖 Prompt for AI Agents 
In src/main/main.ts around line 38, calling
setappFramework.SetappManager.shared.reportUsageEvent(setappFramework.SETAPP_USAGE_EVENT.USER_INTERACTION)
at startup is semantically incorrect; change the reported event to an
initialization/launch event (e.g., SETAPP_USAGE_EVENT.APP_LAUNCH or similar) or
remove this startup call and move it into actual user interaction handlers
(mouse/keyboard/menu/action listeners) so USER_INTERACTION is only reported when
real user input occurs; update the single call accordingly and ensure any new
event constant exists and is imported/used consistently.

log.log("[SETAPP] integration complete")
}
Comment on lines +33 to +38
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Add error handling for optional dependency.

The dynamic require("@setapp/framework-wrapper") at line 37 will crash the application if the optional dependency is not installed. Since @setapp/framework-wrapper is listed as an optionalDependency in release/app/package.json, it may not be present in all environments.

Apply this diff to add proper error handling:

 if (process.env.IS_SETAPP_BUILD === "true") {
   log.log("[SETAPP] build identified")
-  const setappFramework = require("@setapp/framework-wrapper");
-  setappFramework.SetappManager.shared.reportUsageEvent(setappFramework.SETAPP_USAGE_EVENT.USER_INTERACTION);
-  log.log("[SETAPP] integration complete")
+  try {
+    const setappFramework = require("@setapp/framework-wrapper");
+    setappFramework.SetappManager.shared.reportUsageEvent(setappFramework.SETAPP_USAGE_EVENT.USER_INTERACTION);
+    log.log("[SETAPP] integration complete")
+  } catch (error) {
+    log.error("[SETAPP] Failed to initialize Setapp framework:", error);
+  }
 }
📝 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.

Suggested change
if (process.env.IS_SETAPP_BUILD === "true") {
log.log("[SETAPP] build identified")
const setappFramework = require("@setapp/framework-wrapper");
setappFramework.SetappManager.shared.reportUsageEvent(setappFramework.SETAPP_USAGE_EVENT.USER_INTERACTION);
log.log("[SETAPP] integration complete")
}
if (process.env.IS_SETAPP_BUILD === "true") {
log.log("[SETAPP] build identified")
try {
const setappFramework = require("@setapp/framework-wrapper");
setappFramework.SetappManager.shared.reportUsageEvent(setappFramework.SETAPP_USAGE_EVENT.USER_INTERACTION);
log.log("[SETAPP] integration complete")
} catch (error) {
log.error("[SETAPP] Failed to initialize Setapp framework:", error);
}
}
🤖 Prompt for AI Agents 
In src/main/main.ts around lines 35 to 40 the dynamic require of
"@setapp/framework-wrapper" can throw if the optional dependency is not
installed; wrap the require and all usage of setappFramework in a try/catch
block so a missing module or runtime error won’t crash the app, log a clear
warning on failure (including the error message) and continue without calling
SetappManager; optionally guard calls with a truthy check (e.g., if
(setappFramework && setappFramework.SetappManager)) before invoking
reportUsageEvent.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify webpack bundling of optional dependency.

The dynamic require() at line 37 may not work correctly with webpack bundling. Webpack might not include optional dependencies in the bundle, or may require additional configuration. Please verify that @setapp/framework-wrapper is correctly bundled and accessible at runtime in production builds.

Run the following script to check if the Setapp framework is properly bundled in production builds:

🏁 Script executed:

#!/bin/bash
# Description: Verify that @setapp/framework-wrapper is available in production builds

# Check if the package is present in release/app/node_modules
if [ -d "release/app/node_modules/@setapp/framework-wrapper" ]; then
  echo "✓ @setapp/framework-wrapper found in release/app/node_modules"
  ls -la release/app/node_modules/@setapp/framework-wrapper/
else
  echo "✗ @setapp/framework-wrapper NOT found in release/app/node_modules"
fi

# Check webpack externals configuration
echo "Checking webpack externals configuration..."
rg -n "externals" .erb/configs/webpack.config.base.ts -A 5

# Check if @setapp/framework-wrapper is in the externals list
echo "Verifying if @setapp/framework-wrapper is externalized..."
rg -n "@setapp/framework-wrapper" release/app/package.json

Length of output: 429

🏁 Script executed:

#!/bin/bash
# Check if @setapp/framework-wrapper is in root node_modules
echo "=== Checking root node_modules ==="
if [ -d "node_modules/@setapp/framework-wrapper" ]; then
  echo "✓ @setapp/framework-wrapper found in root node_modules"
else
  echo "✗ @setapp/framework-wrapper NOT found in root node_modules"
fi

# Check main package.json
echo ""
echo "=== Checking root package.json for @setapp/framework-wrapper ==="
rg -n "@setapp/framework-wrapper" package.json

# Check release/app package.json structure
echo ""
echo "=== Checking release/app/package.json dependencies section ==="
rg -n "dependencies" release/app/package.json -A 20 | head -30

# Check webpack config for how externals are handled in the actual build
echo ""
echo "=== Checking if there are webpack plugins or special handling for externals ==="
rg -n "externals|optional|@setapp" .erb/configs/ -B 2 -A 2

Length of output: 1673

🏁 Script executed:

#!/bin/bash
# Check the actual code context around the require statement
echo "=== Checking code context for error handling ==="
rg -n "require.*@setapp" src/main/main.ts -B 10 -A 5

# Check if there's a try-catch or conditional check
echo ""
echo "=== Checking for error handling patterns ==="
rg -n "try|catch|optional|IS_SETAPP_BUILD" src/main/main.ts -B 2 -A 2

# Check webpack config to see if optionalDependencies should be included
echo ""
echo "=== Full webpack externals configuration ==="
rg -n "externals" .erb/configs/webpack.config.base.ts -B 5 -A 10

Length of output: 2721

🏁 Script executed:

#!/bin/bash
# Check the actual code context around the require statement
echo "=== Checking code context for error handling ==="
rg -n "require.*@setapp" src/main/main.ts -B 10 -A 5

# Check if there's a try-catch or conditional check
echo ""
echo "=== Checking for error handling patterns ==="
rg -n "try|catch|optional|IS_SETAPP_BUILD" src/main/main.ts -B 2 -A 2

# Check webpack config to see if optionalDependencies should be included
echo ""
echo "=== Full webpack externals configuration ==="
rg -n "externals" .erb/configs/webpack.config.base.ts -B 5 -A 10

Length of output: 2721

Wrap require() in try-catch or update webpack config to handle optional dependencies.

The code lacks error handling for a dynamic require of an optional dependency. The package @setapp/framework-wrapper is listed in release/app/package.json as an optionalDependency (line 27) but the webpack configuration at .erb/configs/webpack.config.base.ts only marks dependencies as externals (line 7-10), excluding optional ones. This means webpack will attempt to bundle the package, but since it's not installed in node_modules, the require at line 37 will fail at runtime without error handling.

Fix one of:

  • Wrap the require and Setapp calls in a try-catch block, or
  • Update webpack externals config to include optionalDependencies alongside dependencies
🤖 Prompt for AI Agents 
In src/main/main.ts around lines 35 to 40, the dynamic require of
"@setapp/framework-wrapper" is unprotected and will throw at runtime if the
optional dependency is absent; wrap the require and subsequent SetappManager
calls in a try-catch that safely logs or ignores the failure, ensuring the app
continues to run, or alternatively update .erb/configs/webpack.config.base.ts to
treat optionalDependencies as externals alongside dependencies so the module is
not bundled and runtime require behaves as expected.



// Init remote so that it could be consumed in renderer
const remote = require("@electron/remote/main");
remote.initialize();
Expand Down Expand Up @@ -427,15 +435,7 @@ app.on("window-all-closed", () => {
app
.whenReady()
.then(() => {
if (process.env.IS_SETAPP_BUILD === "true") {
log.log("[SETAPP] build identified")
const setappFramework = require("@setapp/framework-wrapper");
setappFramework.SetappManager.shared.reportUsageEvent(setappFramework.SETAPP_USAGE_EVENT.USER_INTERACTION);
log.log("[SETAPP] integration complete")
}

app.on("activate", () => {

// On macOS it's common to re-create a window in the app when the
// dock icon is clicked and there are no other windows open.
if (BrowserWindow.getAllWindows().length === 0) {
Expand Down