-
-
Notifications
You must be signed in to change notification settings - Fork 31
chore: more configuration of typescript transpile module #228
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
chore: more configuration of typescript transpile module #228
Conversation
WalkthroughRefactored script execution to accept an options object with explicit imports, enhanced TypeScript/JS transpilation, VM execution with dynamic import support and a 1s timeout, and richer error messages. Updated call sites to the new signature. Also enabled sourcemaps in the core debug build script. No public API surface changes besides execScript signature. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller as tryPageMetaFromMacro
participant Exec as execScript(options)
participant TS as ts.transpileModule
participant VM as Node VM
participant Mod as Module
Caller->>Exec: { imports[], code, filename }
Exec->>TS: Transpile (ES2022 → CJS, relaxed checks)
TS-->>Exec: generated JS
Exec->>VM: create context (require, module, exports, import(), __filename)
Exec->>VM: run wrapped module (1s timeout)
VM-->>Mod: module.exports / default
alt default is function
Exec->>Mod: invoke default()
Mod-->>Exec: result
else value
Mod-->>Exec: value
end
Exec-->>Caller: result
note right of Exec: On error → throw with filename + generated JS
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
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
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 1
🧹 Nitpick comments (1)
packages/core/src/utils.ts (1)
121-121: Consider making timeout configurable.The 1-second timeout is hardcoded, which might be insufficient for complex scripts or slow systems. Consider making this configurable through the options parameter.
-export async function execScript(options: { imports: string[], code: string, filename: string }): Promise<any> { - const { imports = [], code, filename } = options +export async function execScript(options: { imports: string[], code: string, filename: string, timeout?: number }): Promise<any> { + const { imports = [], code, filename, timeout = 1000 } = optionsAnd update the VM execution:
await script.runInNewContext(vmContext, { - timeout: 1000, // 设置超时避免长时间运行 + timeout, // 设置超时避免长时间运行 })
📜 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.
📒 Files selected for processing (3)
packages/core/package.json(1 hunks)packages/core/src/page.ts(1 hunks)packages/core/src/utils.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/core/src/page.ts (2)
packages/core/src/utils.ts (1)
execScript(73-138)packages/core/src/types.ts (1)
UserPageMeta(147-153)
🔇 Additional comments (8)
packages/core/package.json (1)
53-53: Good addition for debugging support.Adding
--sourcemapto the debug script is a valuable improvement that will help with debugging the transpiled TypeScript code, especially given the enhanced transpilation configuration in this PR.packages/core/src/page.ts (1)
111-115: API signature change looks good.The refactored call to
execScriptusing an options object is cleaner and more maintainable than the previous concatenated string approach. This properly separates imports from the actual code being executed.packages/core/src/utils.ts (6)
64-72: Well-documented function with clear parameter descriptions.The JSDoc comment clearly explains the function's purpose and each parameter, which improves code maintainability.
136-136: Enhanced error reporting is helpful for debugging.Including the filename, error message, and generated JS code in the error message will significantly help with debugging transpilation and execution issues.
82-101: Comprehensive TypeScript compiler configuration.The detailed compiler options provide good flexibility by disabling strict checks while maintaining CommonJS compatibility. The ES2022 target is a reasonable choice for modern Node.js environments.
128-133: Consider async function handling.The code checks if the result is a function and executes it, but doesn't handle the case where the function itself might be async and return a Promise.
// 如果是函数,执行函数并返回其返回值 if (typeof result === 'function') { - return Promise.resolve(result()) + const fnResult = result() + return Promise.resolve(fnResult) }This ensures that if
result()returns a Promise, it will be properly awaited.Likely an incorrect or invalid review comment.
114-114:⚠️ Evaluate dynamic import usage in your VM sandboxThe
import(id)call inside a VM context still uses Node’s native dynamic-import mechanism – it doesn’t invoke yoursandbox.importfunction and will instead:
- Reject with
ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSINGunless you explicitly supply animportModuleDynamicallycallback to yourvm.Script/runIn…Contextoptions (nodejs.org).- Bypass the sandbox entirely, loading modules in the main context and reintroducing any APIs or globals you’ve attempted to omit, which may defeat your isolation guarantees (nodejs.org, codingeasypeasy.com).
Please confirm that you either:
- Provide a proper
importModuleDynamicallyhandler (or usevm.SourceTextModule) to safely wire dynamic imports within the sandbox, or- Remove or replace dynamic
import()calls with a sandbox-friendly loader (e.g. pre-declared whitelisted modules).
78-78: No sanitization needed for AST-derived importsThe
importsarray is populated by extracting and regenerating only staticImportDeclarationnodes from the SFC’s AST—not from arbitrary user input at runtime—so there is no avenue for injection beyond what valid ES import syntax allows. Moreover,execScriptruns in a build‐time VM context with only the extracted imports and transpiled code, meaning this is a controlled environment operating on developer‐authored files. You can safely disregard the suggestion to sanitize these imports.Likely an incorrect or invalid review comment.
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: 0
♻️ Duplicate comments (1)
packages/core/src/utils.ts (1)
89-89: Fix typo in Chinese comment.The Chinese comment contains garbled text "检查" that should be corrected for better readability.
- strictNullChecks: false, // 关闭严格的 null 和 undefined 检查 + strictNullChecks: false, // 关闭严格的 null 和 undefined 检查,允许隐式为 null 或 undefined
🧹 Nitpick comments (1)
packages/core/src/utils.ts (1)
78-78: Consider validating the imports array format.The imports are joined with newlines and prepended to the code. Consider validating that each import statement is properly formatted to prevent syntax errors.
export async function execScript(options: { imports: string[], code: string, filename: string }): Promise<any> { const { imports = [], code, filename } = options + + // Validate import statements format + for (const importStmt of imports) { + if (!importStmt.trim().startsWith('import') && !importStmt.trim().startsWith('const') && !importStmt.trim().startsWith('var') && !importStmt.trim().startsWith('let')) { + throw new Error(`Invalid import statement in ${filename}: ${importStmt}`) + } + } let jsCode: string = ''
📜 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.
📒 Files selected for processing (1)
packages/core/src/utils.ts(1 hunks)
🔇 Additional comments (7)
packages/core/src/utils.ts (7)
73-74: LGTM! Clean function signature refactor.The function signature change from separate parameters to an options object improves API ergonomics and makes the function more maintainable as it grows.
81-103: Comprehensive TypeScript transpilation configuration looks good.The TypeScript compiler options are well-configured for VM execution:
- Appropriate target (ES2022) and module (CommonJS) settings
- Relaxed type checking for runtime flexibility
- Good interoperability settings with
esModuleInteropandallowJs- Performance optimizations with
skipLibCheckandremoveCommentsThe configuration strikes a good balance between functionality and permissiveness for script execution.
108-115: Well-designed VM context with comprehensive runtime support.The VM context provides:
- Standard Node.js globals (
__filename,__dirname)- Module system support (
require,module,exports)- Dynamic import capability through the
importfunctionThis creates a robust execution environment for the transpiled scripts.
120-122: Good timeout protection for script execution.The 1000ms timeout prevents long-running or infinite loops from blocking the execution. This is a sensible default for configuration scripts.
125-133: Smart result handling with function execution support.The logic correctly handles both function and non-function exports:
- Executes functions and returns their result
- Returns non-function values directly
- Properly handles async execution with Promise.resolve
This provides flexibility for different script patterns.
135-137: Excellent error reporting with context information.The error handling provides comprehensive debugging information:
- Filename for error location
- Original error message
- Generated JavaScript code for debugging transpilation issues
This will significantly help with debugging script execution failures.
64-72: Well-documented function with clear parameter descriptions.The JSDoc comments clearly explain the function's purpose, parameters, and return behavior. The bilingual documentation (Chinese and code comments) is helpful for the target audience.
Description 描述
增加更多 ts 转译的配置
Summary by CodeRabbit