-
Notifications
You must be signed in to change notification settings - Fork 214
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
feat(nxls): decouple daemon lifecycle from nxls lifecycle #2339
Conversation
MaxKless
commented
Dec 3, 2024
- log enabled
- changes
- start daemon separately & dont kill on shutdown
☁️ Nx Cloud ReportCI is running/has finished running commands for commit b97a01d. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 6 targets
Sent with 💌 from NxCloud. |
3b86f71
to
b97a01d
Compare
@@ -134,6 +137,14 @@ export class NxlsWrapper { | |||
|
|||
this.process?.removeListener('exit', this.earlyExitListener); | |||
|
|||
try { | |||
execSync(`npx nx@${version ?? defaultVersion} daemon --stop`, { |
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.
Should there be a windowsHide here as well?
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.
sure, not important tho as it's just the e2e
@@ -102,7 +102,7 @@ export class NxlsClient { | |||
} | |||
|
|||
public async refreshWorkspace() { | |||
window.withProgress( | |||
await window.withProgress( |
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.
Does this fix the spamming refresh issue?
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.
yes!