Skip to content

Conversation

@sela0629
Copy link

@sela0629 sela0629 commented Aug 23, 2025

English | 简体中文

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Built its own designer, fully self-validated

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Background and solution

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

修复 #1521 问题,请采纳,避免每次安装新版本需要执行patch 兼容错误。

Summary by CodeRabbit

  • Bug Fixes
    • Prevented a rare initialization error on the Canvas page by ensuring browser-safe environment variables, improving startup reliability and compatibility with certain integrations.
  • Chores
    • Updated the Canvas initialization to include a lightweight environment shim for better cross-environment stability.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 23, 2025

Walkthrough

A script tag was inserted into packages/canvas/init-canvas/canvas.html to define window.process with an empty env object. It was placed after the meta viewport tag and before the IMPORT_MAP/IMPORT_STYLE blocks. No exported or public entities were changed.

Changes

Cohort / File(s) Summary of Changes
Canvas HTML bootstrapping
packages/canvas/init-canvas/canvas.html
Inserted a head script initializing window.process = { env: {} } between the viewport meta and import blocks.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A nibble of script in the moonlit DOM,
I set env to {}, calm and calm.
Hops through head, before imports sing,
A tiny tweak—what joy it’ll bring!
Thump-thump, ship it—springy wing. 🐇✨

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the bug Something isn't working label Aug 23, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
packages/canvas/init-canvas/canvas.html (2)

7-11: Consider defaulting NODE_ENV or documenting why it’s omitted.

If any code relies on process.env.NODE_ENV at runtime (without build-time replacement), set a safe default like 'production'. Otherwise, add a brief comment noting that it’s intentionally not set.

Option A — set a default:

-        var env = (existing.env && typeof existing.env === 'object') ? existing.env : {};
+        var env = (existing.env && typeof existing.env === 'object') ? existing.env : {};
+        if (env.NODE_ENV == null) env.NODE_ENV = 'production';

Option B — document the choice:

-      // Minimal, non-invasive polyfill so browser code expecting process.env doesn't crash.
+      // Minimal, non-invasive polyfill so browser code expecting process.env doesn't crash.
+      // Intentionally not setting env.NODE_ENV here; bundlers should replace it at build-time.

7-11: Ensure inline script compatibility under strict CSP

We’ve confirmed that:

  • The canvas.html inline snippet (lines 7–11) remains:
    <script>
      window.process = { env: {} };
    </script>
  • There are no CSP definitions in your HTML files or server-side code that would currently block inline scripts. The only CSP-related code is the upgradeHttpsPlugin, which injects a Content-Security-Policy: upgrade-insecure-requests meta tag into index.html, preview.html, and previewApp.html when in alpha or prod mode—this does not affect canvas.html.
  • No references to unsafe-inline, no <meta http-equiv="Content-Security-Policy"> tags outside the plugin, and no setHeader('Content-Security-Policy', …) or Helmet middleware usage were found in the repo.

However, if your deployment environment enforces a stricter CSP (e.g. disallowing inline scripts), this snippet will be blocked. To future-proof against that:

• Move this code into a tiny external JS file (e.g. init-canvas.js), and include it via

<script src="init-canvas.js" integrity="<hash>" nonce="<nonce>"></script>

or bind it with the nonce/hash you provision in your CSP.
• Alternatively, compute and inject an sha256-… hash of the inline block into your CSP policy.

Please also verify with your infrastructure/configuration team whether any CSP headers are applied at the server or CDN layer that could affect inline scripts.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 7f939c1 and e05caa5.

📒 Files selected for processing (1)
  • packages/canvas/init-canvas/canvas.html (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: gene9831
PR: opentiny/tiny-engine#1041
File: packages/plugins/datasource/src/DataSourceList.vue:138-138
Timestamp: 2025-01-14T10:06:25.508Z
Learning: PR #1041 in opentiny/tiny-engine is specifically for reverting Prettier v3 formatting to v2, without any logical code changes or syntax improvements.
🔇 Additional comments (1)
packages/canvas/init-canvas/canvas.html (1)

7-11: Approve process injection as safe in canvas.html

All occurrences of process.cwd() and similar Node-only properties are confined to build and test scripts, which run in Node (where window.process isn’t present), so they won’t be affected by the injected window.process. No browser-side modules perform typeof process checks or access Node-only fields.

• packages/vue-generator/test/testcases/full/index.js: uses process.cwd() in test runner
• packages/build/vite-config/src/vite-plugins/treeShakingPlugin.js: uses process.cwd() for path resolution
• packages/build/vite-config/src/vite-plugins/devAliasPlugin.js: uses process.cwd() for base paths
• packages/build/vite-config/src/vite-plugins/cdnCopyPlugin.js: uses process.cwd() for file copying
• packages/build/vite-config/src/default-config.js: uses process.cwd() for HTML entry points
• packages/build/vite-config/src/localCdnFile/importMapLocalPlugin.js: uses process.cwd() for import-map lookup

No front-end code depends on Node-only process fields—this quick fix can be merged.

Comment on lines +7 to +11
<script>
window.process = {
env : {}
};
</script>
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid clobbering globals; make the polyfill minimal and browser-explicit.

Unconditionally overwriting window.process risks breaking libraries that use typeof process !== 'undefined' as a Node-runtime signal. Guard the assignment, preserve any existing env, and set process.browser = true to reduce false “Node” detections in the browser.

Apply this diff to make the polyfill non-invasive:

-    <script>
-      window.process = {
-        env : {}
-      };
-    </script>
+    <script>
+      // Minimal, non-invasive polyfill so browser code expecting process.env doesn't crash.
+      (function () {
+        var w = window;
+        var existing = (w.process && typeof w.process === 'object') ? w.process : {};
+        var env = (existing.env && typeof existing.env === 'object') ? existing.env : {};
+        // Do not overwrite existing keys; just ensure objects exist.
+        existing.env = env;
+        // Explicitly mark as browser to avoid libraries mistaking this for Node.
+        existing.browser = true;
+        w.process = existing;
+      })();
+    </script>
📝 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
<script>
window.process = {
env : {}
};
</script>
<script>
// Minimal, non-invasive polyfill so browser code expecting process.env doesn't crash.
(function () {
var w = window;
var existing = (w.process && typeof w.process === 'object') ? w.process : {};
var env = (existing.env && typeof existing.env === 'object') ? existing.env : {};
// Do not overwrite existing keys; just ensure objects exist.
existing.env = env;
// Explicitly mark as browser to avoid libraries mistaking this for Node.
existing.browser = true;
w.process = existing;
})();
</script>
🤖 Prompt for AI Agents
In packages/canvas/init-canvas/canvas.html around lines 7 to 11, the inline
script unconditionally overwrites window.process which can break libs checking
for Node; instead only create window.process if it doesn't exist, ensure
process.env is preserved or initialized to an empty object if missing, and add
process.browser = true so libraries detect a browser environment; do not remove
existing properties — merge/assign env rather than clobbering the whole object.

@chilingling
Copy link
Member

export default defineConfig({

在这里加:

defineConfig({
     define: {
        process: {
          env: {}
        }
    },
})

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically.


export default defineConfig({

Add here:

defineConfig({
     define: {
        process: {
          env: {}
        }
    },
})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants