-
-
Notifications
You must be signed in to change notification settings - Fork 2
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: eslintInstance may not be initialized when calling lintFiles in the worker #40
Conversation
修复在worker模式下因异步问题导致经常报错 Cannot read properties of undefined (reading 'lintFiles')
WalkthroughThe changes involve restructuring the initialization logic for ESLint in the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 0
Outside diff range and nitpick comments (1)
packages/core/src/worker.ts (1)
Line range hint
31-48
: Well-structured initialization function with room for improvementThe new
init
function is a well-structured approach to handling the ESLint initialization. It properly sets up theeslintInstance
,formatter
, andoutputFixes
, effectively addressing the issue of undefinedeslintInstance
. The use ofresolve()
ensures that the Promise is fulfilled after initialization, which is crucial for the asynchronous flow.Consider adding error handling to the
init
function, especially around theinitializeESLint
call and thelintOnStart
operation. This would make the initialization process more robust. For example:const init = async () => { try { debug("Initialize ESLint"); const result = await initializeESLint(options); eslintInstance = result.eslintInstance; formatter = result.formatter; outputFixes = result.outputFixes; resolve(); if (options.lintOnStart) { debug("Lint on start"); await lintFiles({ files: options.include, eslintInstance, formatter, outputFixes, options, }); } } catch (error) { debug(`Initialization error: ${error}`); // Consider how to handle initialization failures } };
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/core/src/worker.ts (2 hunks)
Additional comments not posted (3)
packages/core/src/worker.ts (3)
27-30
: Excellent addition of asynchronous control flowThe introduction of a Promise and its resolve function is a good approach to manage the asynchronous initialization of ESLint. This change directly addresses the issue of
eslintInstance
being undefined in worker mode by ensuring that the initialization is complete before any linting operations are performed.
52-52
: Critical fix for race conditionThe addition of
await p;
at the beginning of the message event handler is a crucial fix. This ensures that the ESLint instance is fully initialized before processing any messages, effectively resolving the race condition that was causing theeslintInstance
to be undefined.This change directly addresses the issue mentioned in the PR objectives and should prevent the "Cannot read properties of undefined (reading 'lintFiles')" error.
Line range hint
27-52
: Overall excellent solution to the reported issueThe changes implemented in this file effectively address the problem of
eslintInstance
being undefined in worker mode. The introduction of a Promise-based initialization process and the restructuring of the code provide a robust solution to the race condition that was causing the issue.Key improvements:
- Asynchronous control flow with Promise
- Well-structured
init
function- Proper sequencing of initialization and message processing
These changes should resolve the "Cannot read properties of undefined (reading 'lintFiles')" error and improve the overall reliability of the plugin in worker mode.
While there's room for minor improvements in error handling, the core issue has been successfully addressed. Great work on this fix!
请先提供一个最小复现demo |
|
由于 cjs 里面无法做到顶层 await,所以只能用 iife 来处理。当 worker 线程启动时,它会执行代码,直到到达 当你设置了 server.open 为 true,vite 会自动预热入口和对应的相关文件,这时候就会导致主线程在 目前的修复方案我认为不太好,可以参考 https://github.com/nabla/vite-plugin-eslint/blob/main/src/worker.cjs#L12-L25 改一下吗?谢谢 |
已调整 |
commit: |
Thank you |
Description 描述
修复在worker模式下因异步问题导致经常报错 Cannot read properties of undefined (reading 'lintFiles')
Linked Issues 关联的 Issues
Additional context 额外上下文
Summary by CodeRabbit
New Features
Bug Fixes