[Fix] Desktop Server Build #8625
Conversation
WalkthroughThe pull request introduces multiple modifications to the project's build process and CORS configuration. In the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎ To accept the risk, merge this PR and you will not be notified again.
Next stepsWhat is protestware?This package is a joke, parody, or includes undocumented or hidden behavior unrelated to its primary function. Consider that consuming this package may come along with functionality unrelated to its primary purpose. Take a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/core/src/lib/graphql/graphql-helper.ts (2)
29-38: Use a central constant or config for allowed HTTP methods
Defining the list of HTTP methods in an array and then joining them is an improvement in terms of readability. However, consider placing this list in a shared constant or configuration file to avoid duplication across multiple files and ensure maintainability if new methods are introduced.
39-52: Revisit the consistency of allowed headers
The array-based headers approach improves clarity. Ensure that CORS-related arrays remain consistent across different modules, and that new headers (like custom security or analytics headers) are added here as needed. You may want to centralize these header arrays, similar to the HTTP methods recommendation, for easier maintenance.packages/core/src/lib/core/core.module.ts (2)
16-18: Remove or guard debug logs before production
The console log statements are helpful for debugging but might clutter production logs. Consider wrapping them with a condition (e.g., an environment check) or removing them entirely if no longer needed.
40-53: Centralize common CORS header arrays
The headers array is appropriate and consistent with other updates. If possible, extract it to a dedicated config to avoid duplication across different modules.packages/core/src/lib/bootstrap/index.ts (1)
109-117: Adopt a common array-based method definition
Using arrays for the methods and then joining them improves maintainability. As previously recommended, consider extracting these into a shared constant for reusability across the application.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
package.json(2 hunks)packages/core/src/lib/bootstrap/index.ts(1 hunks)packages/core/src/lib/core/core.module.ts(3 hunks)packages/core/src/lib/graphql/graphql-helper.ts(1 hunks)
🔇 Additional comments (5)
packages/core/src/lib/core/core.module.ts (2)
30-39: Maintain consistent CORS methods across the codebase
Similar to other modules, defining methods in an array and then joining them is good. Keep these arrays synchronized with any future changes, such as new or deprecated HTTP methods.
68-78: Good JSDoc usage with explicit return types
Declaring : void clarifies the method has no return value, and the detailed JSDoc is beneficial for maintenance. This approach is consistent with TypeScript best practices.
packages/core/src/lib/bootstrap/index.ts (1)
118-131: Validate common usage of allowed headers
The approach matches the pattern in other files. Double-check that updates to allowed headers in the future remain in sync across all modules by centralizing them in a single configuration constant or utility.
package.json (2)
250-250: New “build:package:gauzy:prod” script
This consolidated production build command looks well-organized. Ensure that dependent scripts are all tested and verified to confirm no necessary steps were omitted when bundling in production mode.
273-273: Updated “build:gauzy-server” script
Combines multiple steps to streamline the server’s production build, including packaging and icon generation. Verify that each step (packaging, generating icons, building the server, etc.) has been tested to ensure that interdependencies work correctly.
|
View your CI Pipeline Execution ↗ for commit 37ee771.
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignore(1 hunks)apps/server/src/package.json(1 hunks)package.json(7 hunks)
🔇 Additional comments (4)
package.json (4)
30-34: New configuration scripts enhance build process clarity!
The addition of environment-specific electron configuration scripts improves the build process organization and maintainability.
255-257: New gauzy package build scripts look good!
The new production build scripts properly separate concerns and maintain consistency with other build configurations.
286-286: File copy and icon generation scripts are properly configured!
The new scripts for copying files and generating icons maintain consistency across different build targets.
Also applies to: 307-307
273-273: Server build scripts have been properly updated!
The changes to build:gauzy-server and build:gauzy-api-server scripts now:
- Use the correct environment configurations
- Include all necessary build steps
- Maintain proper order of operations
Let's verify the build script dependencies:
Also applies to: 297-297
✅ Verification successful
All build script dependencies are present and correctly configured
All referenced scripts in the build:gauzy-server command exist in package.json and form a valid build chain:
copy-files-i18n-server: Downloads translations for serverpostinstall.electron: Installs electron dependencies and runs postinstall scriptconfig:server:prod: Configures electron for production server environmentbuild:package:gauzy:prod: Builds all required packages in correct orderpack:server: Runs pack command for servergenerate:icons:server: Generates icons for serverprepare:gauzy-server: Runs electron postinstall and compiles server TypeScriptcopy-files-gauzy-server: Copies GraphQL schema files to server dist
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all referenced scripts exist
# Extract script names from the build commands
scripts=$(echo "copy-files-i18n-server postinstall.electron config:server:prod build:package:gauzy:prod pack:server generate:icons:server prepare:gauzy-server copy-files-gauzy-server" | tr ' ' '\n')
# Check if each script exists in package.json
for script in $scripts; do
rg "\"$script\":" package.json
done
Length of output: 1870
.gitignore
Outdated
| # Ignore all environment files | ||
| **/environments/environment.ts | ||
| **/environments/environment.prod.ts | ||
|
|
||
| /apps/desktop-timer/src/environments/environment.prod.ts | ||
| /apps/desktop-timer/src/environments/environment.ts | ||
|
|
||
| /apps/desktop/src/environments/environment.prod.ts | ||
| /apps/desktop/src/environments/environment.ts | ||
|
|
||
| /apps/server/src/environments/environment.prod.ts | ||
| /apps/server/src/environments/environment.ts | ||
| # Exclude specific file from being ignored | ||
| !/packages/config/src/lib/environments/environment.ts | ||
| !/packages/config/src/lib/environments/environment.prod.ts |
There was a problem hiding this comment.
💡 Codebase verification
Environment file patterns need adjustment to avoid excluding critical files
The current gitignore patterns would unintentionally ignore critical environment files that are currently tracked in the repository, specifically:
- Environment files in
apps/desktop-api/src/environments/ - Environment files in
apps/server-api/src/environments/ - Environment files in
packages/ui-config/src/lib/environments/
The current patterns are too broad and would ignore all environment files except those in the config package. I recommend:
- Making the ignore patterns more specific to target only development environment files
- Or explicitly excluding the additional critical paths that should remain tracked
🔗 Analysis chain
Environment file patterns look good!
The new patterns provide better standardization for environment file handling while ensuring critical config files remain tracked.
Let's verify the patterns don't accidentally exclude any critical files:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify environment file patterns
# Check which environment files would be ignored
git check-ignore **/environments/environment.ts
# Verify config package files are not ignored
git check-ignore packages/config/src/lib/environments/environment.ts
Length of output: 122
Script:
#!/bin/bash
# Let's try a different approach to verify environment files
# First, find all environment files
echo "All environment files in the repository:"
fd "environment\..*\.ts$"
echo -e "\nEnvironment files in config package:"
fd "environment\..*\.ts$" packages/config/
echo -e "\nChecking git status for these files:"
# This will show if any environment files are already tracked
git ls-files "*environment*.ts"
Length of output: 1953
| "@gauzy/contracts": "file:../../../dist/packages/contracts", | ||
| "@gauzy/config": "file:../../../dist/packages/config", | ||
| "@gauzy/core": "file:../../../dist/packages/core", | ||
| "@gauzy/desktop-lib": "file:../../../dist/packages/desktop-lib", | ||
| "@gauzy/desktop-window": "file:../../../dist/packages/desktop-window", | ||
| "@gauzy/plugin": "file:../../../dist/packages/plugin", |
There was a problem hiding this comment.
💡 Codebase verification
Dependencies should not be removed as they are actively used in the codebase
The verification reveals that @gauzy/desktop-lib and @gauzy/desktop-window packages are still actively imported and used across multiple files in the codebase:
apps/server/src/index.tsimports both packagesapps/server/src/preload/desktop-server-api.tsimportsdesktop-lib- Several other components also depend on these packages:
- apps/server-api
- apps/desktop
- apps/desktop-timer
- packages/desktop-lib
These dependencies should be retained in apps/server/src/package.json to maintain the functionality of the server application.
🔗 Analysis chain
Dependencies cleanup looks good!
The changes appropriately:
- Add the required contracts dependency
- Remove unused desktop-specific dependencies
- Maintain better organization of dependencies
Let's verify the dependency changes don't break any imports:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify no broken imports after dependency changes
# Search for any remaining imports of removed packages
rg "from ['|\"]@gauzy/(desktop-lib|desktop-window)" --type ts
Length of output: 1232
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
package.json (1)
103-103: Consider refactoring build scripts for better maintainabilityThe build scripts are quite complex with multiple steps. Consider extracting common build steps into separate scripts to reduce duplication and improve maintainability.
Example approach:
+ "common:build:steps": "cross-env NODE_ENV=production yarn run postinstall.electron && yarn run build:package:all", + "common:electron:build": "npm config set cache .cache && yarn electron-builder -c.electronVersion=30.0.1", "build:desktop": "yarn run common:build:steps && yarn run config:desktop:prod && yarn run pack:desktop && yarn run generate:icons:desktop --environment=prod && yarn ng:prod run gauzy:desktop-ui --base-href ./ && yarn run prepare:desktop && yarn ng:prod run api:desktop-api && yarn ng:prod build desktop-api --output-path=dist/apps/desktop/desktop-api && yarn run copy-files-desktop && yarn run copy-assets-gauzy"This would make the scripts more maintainable and reduce the risk of inconsistencies between different build targets.
Also applies to: 255-255, 259-259, 273-273, 297-297
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/server/src/package.json(2 hunks)package.json(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/server/src/package.json
🔇 Additional comments (3)
package.json (3)
30-34: Well-structured configuration scripts!
The new electron configuration scripts follow a consistent pattern and properly separate concerns for different build targets. Using a single source script (electron.env.ts) with different parameters is a maintainable approach.
286-286: Well-organized file operation scripts!
The scripts for copying files and generating icons are well-structured and follow a consistent pattern. Each operation has a clear purpose and is properly separated by build target.
Also applies to: 307-307
103-103: Verify the build script changes across different environments
While the build script changes look correct, please ensure they work as intended across different environments (Windows, Linux, Mac).
Run the following script to check for any potential issues in the build commands:
Also applies to: 255-255, 259-259, 273-273, 297-297
✅ Verification successful
Build script changes are well-structured and consistent across environments
The verification results show that:
- The build commands maintain consistent patterns across different environments (Windows, Linux, Mac)
- Environment variables (NODE_ENV) are properly set for production builds
- Copy commands use correct source and destination paths relative to the repository root
- All required dependencies (cross-env, electron-builder) are properly specified
The changes to the build scripts follow the established patterns in the codebase and maintain consistency with other similar commands for different targets (desktop, server, api-server).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the build commands for syntax errors and consistent environment variable usage
# Check for required commands in build scripts
echo "Checking for required commands in build scripts..."
rg --no-filename "(?:yarn run|cross-env|electron-builder)" \
| sort \
| uniq -c \
| awk '{print "Command: " $2 ", Count: " $1}'
# Verify environment consistency
echo "Checking environment variable consistency..."
rg --no-filename "NODE_ENV=\w+" \
| sort \
| uniq -c \
| awk '{print "Env setting: " $2 ", Count: " $1}'
# Check for potential path issues in copy commands
echo "Checking copy commands for potential path issues..."
rg --no-filename "(?:copyfiles|copy-files)" -A 1
Length of output: 30294
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
package.json (1)
274-274: Consider adding error handling to the build stepsWhile the server build script improvements are good, consider adding error handling and status checks between critical build steps to ensure the build process fails fast if any step fails.
-"build:gauzy-server": "cross-env NODE_ENV=production yarn run copy-files-i18n-server && yarn run postinstall.electron && yarn run config:server:prod && yarn run build:package:all:prod && yarn run pack:server && yarn run generate:icons:server --environment=prod && yarn ng:prod build gauzy-server --base-href ./ && yarn run prepare:gauzy-server && yarn ng run gauzy:server-ui && yarn ng run api:gauzy-server-api && yarn run copy-files-gauzy-server", +"build:gauzy-server": "cross-env NODE_ENV=production yarn run copy-files-i18n-server && yarn run postinstall.electron && yarn run config:server:prod && yarn run build:package:all:prod || exit 1 && yarn run pack:server && yarn run generate:icons:server --environment=prod && yarn ng:prod build gauzy-server --base-href ./ || exit 1 && yarn run prepare:gauzy-server && yarn ng run gauzy:server-ui && yarn ng run api:gauzy-server-api && yarn run copy-files-gauzy-server",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
package.json(7 hunks)
🔇 Additional comments (5)
package.json (5)
30-34: Well-structured configuration scripts!
The new electron configuration scripts are well-organized, following a consistent pattern and properly separating concerns for different build targets (desktop, server, desktop-timer).
103-103: Improved desktop build process
The desktop build script has been enhanced with proper configuration and package building steps. The build order ensures all dependencies are properly built before the desktop application.
260-260: Consistent desktop timer build configuration
The desktop timer build script has been updated to follow the same pattern as the main desktop build, ensuring consistent configuration and build steps.
298-298: Consistent API server build configuration
The API server build script has been updated to follow the same pattern as other build scripts, ensuring consistent configuration and build steps.
103-103: Verify build script dependencies
Please verify that the build script dependencies are properly ordered and that there are no circular dependencies between the various build steps.
Also applies to: 260-260, 274-274, 298-298
✅ Verification successful
Let me analyze the build script dependencies from the output. The main build scripts and their dependencies are clearly visible now. Let me generate the final response:
Build script dependencies are properly ordered
The build scripts follow a clear and consistent dependency order:
-
Desktop build script (
build:desktop) properly orders its dependencies:- Environment setup and file copying
- Package builds via
build:package:all - Desktop-specific builds (UI, API)
- Final asset copying
-
Desktop Timer build script (
build:desktop-timer) follows similar pattern -
Server builds (
build:gauzy-server,build:gauzy-api-server) maintain consistent ordering
No circular dependencies were found in the build chain. The package builds are properly sequenced with clear pre/post plugin builds and proper ordering of core components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential circular dependencies in build scripts
echo "Analyzing build script dependencies..."
# Search for cross-references between build scripts
rg -A 5 "build:(desktop|desktop-timer|gauzy-server|gauzy-api-server)" package.json
# Look for potential circular dependencies in build steps
rg "yarn run build:" package.json | sort | uniq
Length of output: 34773
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 CodeRabbit
New Features
@gauzy/contracts.Bug Fixes
Documentation
configuremethod in the CoreModule.Chores
.gitignorepatterns for managing environment files.gauzy-serverproject.