Stage #9325
Stage #9325
Conversation
[Fix] Angular UI Build Speed
* fix: blank page window in agent and remove scrollbar on widget * fix: remove visible overflow * fix: remove unused path alias and clean up global polyfill definition
…9324) * fix: build desktop target env mismatch and agent build repo mismatch * fix: agent app name inconsistent
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
Overview
This PR contains 22 changes across build configuration, GitHub workflows, and UI components. While most changes are minor configuration updates, two critical issues have been identified that will break production builds.
Critical Issues Found
1. Agent Webpack Target Change (Breaks Electron Build)
- File:
apps/agent/config/custom-webpack.config.js - Issue: Changed webpack target from
'electron-renderer'to'web', and added browser polyfills (crypto-browserify, stream-browserify, util, buffer, process/browser) - Impact: This is a breaking change that will cause the Electron Agent app to fail. The app requires Electron-specific module resolution, not browser polyfills
- Severity: CRITICAL - Breaks production builds
2. Build Package Chain Modification (Breaks Dependencies)
- File:
package.json(lines 302-304) - Issue:
build:package:gauzyscripts were significantly changed from building comprehensive backend packages (common, utils, config, plugin, auth, core) to only building UI packages (ui-config, ui-core, ui-auth) - Impact: The API build scripts still reference packages like "common", "utils", "config", "plugin", and "core" which are no longer built. This causes build failures
- Severity: CRITICAL - Breaks API builds
Build Issues
3. Verbose Flag Removal (Consistency Issue)
- File:
package.json - Issue:
--verboseflag was removed from build:gauzy:prod but is still available in build:gauzy:prod:debug - Severity: Minor - Affects build logging consistency
Improvements
4. Gauzy Build Optimization
- File:
apps/gauzy/config/custom-webpack.config.js - Changes: Enabled module concatenation and multi-pass Terser compression
- Impact: Positive - improves build performance
5. Component CSS Enhancement
- File:
packages/desktop-ui-lib/src/lib/always-on/always-on.component.ts - Changes: Added
overflow: hiddenCSS style to body - Impact: Positive - improves always-on window presentation
Configuration Updates
- Updated 7 plugin
project.jsonfiles withimplicitDependencies: []field (Nx best practice) - Updated GitHub workflow environment variables from
DESKTOP_AGENT_APP_*toAGENT_APP_*(consistent naming) - Reorganized
package.jsonoverrides section for electron-rebuild dependencies
Recommendation
DO NOT MERGE - This PR contains critical breaking changes that will prevent the Electron Agent app from building and will break the API build chain. The webpack target change and build script modifications must be reverted before this PR can be merged.
The minor build optimizations and configuration updates are good, but they cannot override the critical issues.
Confidence Score: 0/5
- This PR is NOT safe to merge. It contains two critical breaking changes that will prevent production builds and deployments from working.
- Score of 0 reflects the critical severity of the issues found:
-
Electron Renderer Target Change: Changing from 'electron-renderer' to 'web' target, combined with browser polyfills, will completely break the Electron Agent application. This is a fundamental architectural change that contradicts the app's core design.
-
Build Package Chain Breaking: The modification to build:package:gauzy scripts removes critical backend packages from the build chain, breaking API builds and causing dependency resolution failures.
These are not edge cases or potential issues - they are definite breaking changes that will cause immediate build and runtime failures in production. The PR modifies core build infrastructure in incompatible ways.
The other changes (build optimizations, CSS fixes, workflow updates) are positive, but they are overshadowed by the critical issues.
-
- apps/agent/config/custom-webpack.config.js (CRITICAL: webpack target configuration)
- package.json (CRITICAL: build package scripts and overrides)
- All GitHub workflow files (needs verification after main issues fixed)
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| apps/agent/config/custom-webpack.config.js | 0/5 | CRITICAL: Changed target from 'electron-renderer' to 'web' and added browser polyfills. This breaks Electron app compilation and runtime. The change conflicts with Electron architecture and uses browser APIs instead of Node.js modules. |
| package.json | 1/5 | CRITICAL: build:package:gauzy scripts fundamentally changed, removing critical backend packages from build chain. This breaks API builds. Minor issue: --verbose flag removed inconsistently and package.json override reorganization reduces maintainability. |
| apps/gauzy/config/custom-webpack.config.js | 4/5 | Build optimization improvements: enabled module concatenation and multi-pass compression. These are beneficial performance changes. |
| packages/desktop-ui-lib/src/lib/always-on/always-on.component.ts | 5/5 | Minor CSS fix: Added overflow:hidden to body element for better always-on window presentation. No functional issues. |
Sequence Diagram
sequenceDiagram
participant User as User/CI
participant Build as Build System
participant Webpack as Webpack Config
participant Agent as Agent App
participant API as API Build Chain
User->>Build: yarn build:package:gauzy
Build->>Build: OLD: Builds constants, contracts, common, utils, config, plugin, auth, core
Build->>API: Provides all required packages ✓
User->>Build: yarn build:package:gauzy (NEW)
Build->>Build: NEW: Builds only constants, contracts, ui-config, ui-core, ui-auth
Build->>API: Tries to build API
API-->>Build: ERROR: Missing common, utils, config, plugin, core ✗
User->>Webpack: Build Agent App
Webpack->>Webpack: OLD: Uses electron-renderer target + Node.js modules
Webpack->>Agent: Produces working Electron app ✓
User->>Webpack: Build Agent App (NEW)
Webpack->>Webpack: NEW: Uses web target + browser polyfills
Webpack->>Agent: Module resolution fails ✗
Agent-->>User: BUILD FAILED - Incompatible configuration
| console.log('Building in CircleCI environment'); | ||
| } | ||
|
|
||
| module.exports = { |
There was a problem hiding this comment.
CRITICAL: Changing webpack target from 'electron-renderer' to 'web' will break the Electron application. The agent app is designed to run as an Electron renderer process, not as a web application. This change will cause the app to fail to load properly in Electron.
The 'electron-renderer' target is necessary for Electron apps to properly handle Node.js modules and browser APIs. Using 'web' target will cause module resolution failures and API incompatibilities.
| module.exports = { | |
| target: 'electron-renderer', |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/agent/config/custom-webpack.config.js
Line: 19:19
Comment:
CRITICAL: Changing webpack target from 'electron-renderer' to 'web' will break the Electron application. The agent app is designed to run as an Electron renderer process, not as a web application. This change will cause the app to fail to load properly in Electron.
The 'electron-renderer' target is necessary for Electron apps to properly handle Node.js modules and browser APIs. Using 'web' target will cause module resolution failures and API incompatibilities.
```suggestion
target: 'electron-renderer',
```
How can I resolve this? If you propose a fix, please make it concise.| "build:gauzy:prod": "yarn run config:prod && yarn ng:prod build gauzy -c=production --prod", | ||
| "build:gauzy:prod:debug": "yarn run config:prod && yarn ng:prod build gauzy -c=production --prod --verbose", | ||
| "build:gauzy:prod:docker": "yarn run config:prod && yarn ng:docker build gauzy -c=production --prod", | ||
| "build:gauzy:prod:ci": "yarn run config:prod && yarn --frozen-lockfile --cache-folder ~/.cache/yarn ng:ci build gauzy -c=production --prod", |
There was a problem hiding this comment.
CRITICAL: The build:package:gauzy scripts have been fundamentally changed. The old scripts built a comprehensive set of packages (constants, contracts, common, utils, config, plugin, auth, plugins:pre, core, plugins:post), but the new scripts now build only UI-related packages (constants, contracts, ui-config, ui-core, ui-auth, plugins:pre).
This is a breaking change that removes critical backend packages from the build chain. The API build scripts (build:package:api) still reference packages like "common", "utils", "config", "plugin", and "core" which will no longer be built by build:package:gauzy. This will cause the API builds to fail due to missing dependencies.
The new build scripts are incomplete and don't match the API requirements.
| "build:gauzy:prod": "yarn run config:prod && yarn ng:prod build gauzy -c=production --prod", | |
| "build:gauzy:prod:debug": "yarn run config:prod && yarn ng:prod build gauzy -c=production --prod --verbose", | |
| "build:gauzy:prod:docker": "yarn run config:prod && yarn ng:docker build gauzy -c=production --prod", | |
| "build:gauzy:prod:ci": "yarn run config:prod && yarn --frozen-lockfile --cache-folder ~/.cache/yarn ng:ci build gauzy -c=production --prod", | |
| "build:package:gauzy": "yarn run build:package:constants && yarn run build:package:contracts && yarn run build:package:common && yarn run build:package:utils && yarn run build:package:config && yarn run build:package:plugin && yarn run build:package:auth && yarn run build:package:plugins:pre && yarn run build:package:core && yarn run build:package:plugins:post", | |
| "build:package:gauzy:prod": "yarn run build:package:constants:prod && yarn run build:package:contracts:prod && yarn run build:package:common:prod && yarn run build:package:utils:prod && yarn run build:package:config:prod && yarn run build:package:plugin:prod && yarn run build:package:auth:prod && yarn run build:package:plugins:pre:prod && yarn run build:package:core:prod && yarn run build:package:plugins:post:prod", | |
| "build:package:gauzy:docker": "yarn run build:package:constants:docker && yarn run build:package:contracts:docker && yarn run build:package:common:docker && yarn run build:package:utils:docker && yarn run build:package:config:docker && yarn run build:package:plugin:docker && yarn run build:package:auth:docker && yarn run build:package:plugins:pre:docker && yarn run build:package:core:docker && yarn run build:package:plugins:post:docker", |
Prompt To Fix With AI
This is a comment left during a code review.
Path: package.json
Line: 64:67
Comment:
CRITICAL: The build:package:gauzy scripts have been fundamentally changed. The old scripts built a comprehensive set of packages (constants, contracts, common, utils, config, plugin, auth, plugins:pre, core, plugins:post), but the new scripts now build only UI-related packages (constants, contracts, ui-config, ui-core, ui-auth, plugins:pre).
This is a breaking change that removes critical backend packages from the build chain. The API build scripts (build:package:api) still reference packages like "common", "utils", "config", "plugin", and "core" which will no longer be built by build:package:gauzy. This will cause the API builds to fail due to missing dependencies.
The new build scripts are incomplete and don't match the API requirements.
```suggestion
"build:package:gauzy": "yarn run build:package:constants && yarn run build:package:contracts && yarn run build:package:common && yarn run build:package:utils && yarn run build:package:config && yarn run build:package:plugin && yarn run build:package:auth && yarn run build:package:plugins:pre && yarn run build:package:core && yarn run build:package:plugins:post",
"build:package:gauzy:prod": "yarn run build:package:constants:prod && yarn run build:package:contracts:prod && yarn run build:package:common:prod && yarn run build:package:utils:prod && yarn run build:package:config:prod && yarn run build:package:plugin:prod && yarn run build:package:auth:prod && yarn run build:package:plugins:pre:prod && yarn run build:package:core:prod && yarn run build:package:plugins:post:prod",
"build:package:gauzy:docker": "yarn run build:package:constants:docker && yarn run build:package:contracts:docker && yarn run build:package:common:docker && yarn run build:package:utils:docker && yarn run build:package:config:docker && yarn run build:package:plugin:docker && yarn run build:package:auth:docker && yarn run build:package:plugins:pre:docker && yarn run build:package:core:docker && yarn run build:package:plugins:post:docker",
```
How can I resolve this? If you propose a fix, please make it concise.| "build:gauzy:prod": "yarn run config:prod && yarn ng:prod build gauzy -c=production --prod", | ||
| "build:gauzy:prod:debug": "yarn run config:prod && yarn ng:prod build gauzy -c=production --prod --verbose", | ||
| "build:gauzy:prod:docker": "yarn run config:prod && yarn ng:docker build gauzy -c=production --prod", | ||
| "build:gauzy:prod:ci": "yarn run config:prod && yarn --frozen-lockfile --cache-folder ~/.cache/yarn ng:ci build gauzy -c=production --prod", |
There was a problem hiding this comment.
Inconsistent removal of --verbose flag: Line 64 removes --verbose from build:gauzy:prod but the flag was still present in build:gauzy:prod:debug (line 65). However, checking the git diff, the --verbose flag is removed from all three scripts (build:gauzy:prod, build:gauzy:prod:docker, build:gauzy:prod:ci) inconsistently. This breaks consistency in build output verbosity settings.
If verbosity needs to be reduced for performance, it should be done consistently. Alternatively, if you want to keep the verbose option available, create a separate dedicated debug script instead of inconsistently removing it.
Consider this for consistency across all prod build scripts.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: package.json
Line: 64:67
Comment:
Inconsistent removal of `--verbose` flag: Line 64 removes `--verbose` from build:gauzy:prod but the flag was still present in build:gauzy:prod:debug (line 65). However, checking the git diff, the --verbose flag is removed from all three scripts (build:gauzy:prod, build:gauzy:prod:docker, build:gauzy:prod:ci) inconsistently. This breaks consistency in build output verbosity settings.
If verbosity needs to be reduced for performance, it should be done consistently. Alternatively, if you want to keep the verbose option available, create a separate dedicated debug script instead of inconsistently removing it.
Consider this for consistency across all prod build scripts.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| "locutus": "2.0.30" | ||
| }, | ||
| "electron-rebuild": { | ||
| "node-abi": "3.78.0", | ||
| "@electron/rebuild/node-abi": "3.78.0", | ||
| "app-builder-lib/node-abi": "3.78.0" | ||
| "node-abi": "3.78.0" | ||
| }, | ||
| "@electron/rebuild": { | ||
| "node-abi": "3.78.0" | ||
| }, | ||
| "app-builder-lib": { | ||
| "node-abi": "3.78.0" | ||
| } |
There was a problem hiding this comment.
Reorganized the overrides section to split electron-rebuild related node-abi configurations. While this change works, it creates scattered configuration that was previously centralized. The intent seems to be clarifying which dependencies need the node-abi override, but this reduces maintainability.
Consider adding a comment explaining why @electron/rebuild and app-builder-lib need separate node-abi overrides, or consolidate them back into a single electron-rebuild override block with better documentation.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: package.json
Line: 686:696
Comment:
Reorganized the `overrides` section to split electron-rebuild related node-abi configurations. While this change works, it creates scattered configuration that was previously centralized. The intent seems to be clarifying which dependencies need the node-abi override, but this reduces maintainability.
Consider adding a comment explaining why @electron/rebuild and app-builder-lib need separate node-abi overrides, or consolidate them back into a single electron-rebuild override block with better documentation.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| mainFields: ['es2016', 'browser', 'module', 'main'], | ||
| conditionNames: ['browser', 'import', 'default'], | ||
| fallback: { | ||
| crypto: require.resolve('crypto-browserify'), | ||
| stream: require.resolve('stream-browserify'), | ||
| util: require.resolve('util/'), | ||
| buffer: require.resolve('buffer/'), | ||
| process: require.resolve('process/browser') | ||
| } | ||
| }, |
There was a problem hiding this comment.
Adding browser-specific polyfills (crypto-browserify, stream-browserify, util, buffer, process/browser) to an Electron app's webpack configuration is problematic. Electron apps should use Node.js built-in modules directly, not browser polyfills. This configuration treats the app as if it needs to run in a browser, which conflicts with:
- The electron-renderer environment where native Node.js modules are available
- The use of electron-related externals (electron, electron-log) which are not browser APIs
- The app's actual Electron architecture
This polyfill configuration will:
- Increase bundle size unnecessarily
- Cause module resolution conflicts
- Potentially break native Electron functionality
Remove the fallback polyfill configuration or restore the 'electron-renderer' target. The conditionNames should also be electron-specific, not browser-specific.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/agent/config/custom-webpack.config.js
Line: 22:31
Comment:
Adding browser-specific polyfills (crypto-browserify, stream-browserify, util, buffer, process/browser) to an Electron app's webpack configuration is problematic. Electron apps should use Node.js built-in modules directly, not browser polyfills. This configuration treats the app as if it needs to run in a browser, which conflicts with:
1. The electron-renderer environment where native Node.js modules are available
2. The use of electron-related externals (electron, electron-log) which are not browser APIs
3. The app's actual Electron architecture
This polyfill configuration will:
- Increase bundle size unnecessarily
- Cause module resolution conflicts
- Potentially break native Electron functionality
Remove the fallback polyfill configuration or restore the 'electron-renderer' target. The conditionNames should also be electron-specific, not browser-specific.
How can I resolve this? If you propose a fix, please make it concise.
PR
Please note: we will close your PR without comment if you do not check the boxes above and provide ALL requested information.
Summary by cubic
Fixes the agent blank window and removes the widget scrollbar, while speeding up Angular UI builds and cleaning up stage/prod CI workflows.
Bug Fixes
Build/CI
Written for commit 70292d7. Summary will update on new commits.