-
Notifications
You must be signed in to change notification settings - Fork 326
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.
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
fix: generate code missing template on development runtime #965
fix: generate code missing template on development runtime #965
Conversation
WalkthroughThe pull request introduces modifications to Vite configuration files in two locations: Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
designer-demo/vite.config.js
(1 hunks)packages/engine-cli/template/designer/vite.config.js
(1 hunks)
🔇 Additional comments (1)
designer-demo/vite.config.js (1)
19-19
: Validate that port 8090 configuration is consistent.
If the port is aligned across multiple config files, ensure no conflicts with other local apps to streamline the developer experience.
define: { | ||
process: {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Define process.env
in the Vite configuration to prevent runtime errors
Based on the search results, there are several instances where process.env
is being used across the codebase, particularly:
- Environment checks like
process.env.NODE_ENV
- Configuration values in other Vite configs (e.g., in
packages/vue-generator/test/testcases/generator/expected/appdemo01/vite.config.js
) - Custom environment variables like
VUE_APP_UI_LIB_FULL_STYLE_FILE_URL
To prevent potential runtime errors, you should explicitly define process.env
instead of an empty process
object:
define: {
'process.env': {} // or include specific env variables needed
}
🔗 Analysis chain
Check usage of the process
object.
Declaring an empty process
object might be needed for certain libraries referencing process
. However, if the code depends on process.env
, consider explicitly defining it in order to avoid potential runtime errors.
Here is a script to see if process.env
is directly referenced in your code:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching for references to `process.env` in the repository
rg -A 3 'process\.env'
Length of output: 2516
@@ -15,6 +15,9 @@ | |||
publicDir: path.resolve(__dirname, './public'), | |||
server: { | |||
port: 8090 | |||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Based on the search results, I found that port 8090 is configured in two vite config files:
- packages/engine-cli/template/designer/vite.config.js
- packages/engine-cli/template/preview/vite.config.js
Both files use port 8090 for their development servers. Since these are separate applications (designer and preview), they cannot run simultaneously on the same port. This could lead to port conflicts.
Port 8090 is already in use by multiple applications. Please choose a different port or ensure only one application uses this port at a time.
- The designer application and preview application both try to use port 8090
- Running both simultaneously will cause port conflicts
- Consider using different ports for these applications (e.g., 8090 for designer and 8091 for preview)
- Or ensure only one application is running at a time
🔗 Analysis chain
Confirm that port 8090 is available.
Changing the server port to 8090 can be fine, but ensure there's no port conflict on the target environment, especially if multiple local services run concurrently.
Here is a script to check references in the codebase for "port" settings, in case there are multiple conflicts:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching for any other `server.port` usage in the repository
rg -A 3 $'server:\\s*{\\s*port:'
Length of output: 144
Script:
#!/bin/bash
# Let's try a different approach to search for port configurations
rg -i "port.*=.*8090"
# Also search for other port configurations in the codebase
rg -i "port.*=.*\d{4}"
# Search specifically in vite config files
fd "vite.config" --exec cat {}
Length of output: 66288
Script:
#!/bin/bash
# Let's try a more specific search for port configurations in vite config files
rg -i "port.*=.*8090" --type js
# Also search for any port configurations in vite files
rg -i "port.*=" --type js "vite.config"
# Find and show content of vite config files
fd "vite.config.js" --exec cat {}
Length of output: 66919
define: { | ||
process: {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Empty process
object is insufficient, process.env
is needed
Based on the search results, the codebase shows multiple usages of process.env
, particularly:
- Environment checks like
process.env.NODE_ENV
- Configuration values in mockServer
- Vue template generation
- Block to web component conversion using
process.env.VUE_APP_UI_LIB_FULL_STYLE_FILE_URL
The empty process
object in the Vite config should be updated to include process.env
to support these environment-dependent features.
Example fix:
define: {
'process.env': {}
}
🔗 Analysis chain
Assert that the empty process
fits your needs.
An empty process
object helps avoid build-time errors if certain packages expect a Node-like environment. If your app specifically relies on environment variables, define process.env
or use alternative stubs instead.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching references for `process.*` usage in the code to confirm needed fields
rg -A 3 'process\.'
Length of output: 8527
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit