-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat(core): enhance spinner handling and add spinner to inferred conversion #33031
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
Conversation
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
💡 Enable Vercel Agent with $100 free credit for automated AI reviews |
|
View your CI Pipeline Execution ↗ for commit 7b79a88
☁️ Nx Cloud last updated this comment at |
packages/devkit/src/generators/plugin-migrations/executor-to-plugin-migrator.ts
Outdated
Show resolved
Hide resolved
| stop() { | ||
| this.#ora.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.
The stop() method should check if this.#ora exists before calling stop() on it. Currently, if stop() is called before start(), this.#ora will be undefined and cause a runtime error. Consider adding a null check:
stop() {
if (this.#ora) {
this.#ora.stop();
}
}This pattern is already used correctly in the updateText() method below.
| stop() { | |
| this.#ora.stop(); | |
| } | |
| stop() { | |
| if (this.#ora) { | |
| this.#ora.stop(); | |
| } | |
| } |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
0e4309a to
771d1b5
Compare
771d1b5 to
87e97a2
Compare
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.
Nx Cloud has identified a possible root cause for your failed CI:
The failure analysis reveals that the nx-dev:build-base task failed because Next.js cannot find the directory public/documentation/blog during static page generation for the /blog/[slug] route.
Classification rationale for 'environment_state':
-
No correlation with PR changes: The PR modifies spinner handling utilities, test setup code, documentation markdown files, and ESLint generator configuration. None of these changes involve:
- Deleting or moving the
public/documentation/blogdirectory - Modifying blog-related functionality
- Changing Next.js static generation configuration
- Altering file system operations related to the blog
- Deleting or moving the
-
File system error unrelated to code logic: The error is "ENOENT: no such file or directory" - a fundamental file system issue indicating a missing directory that should exist in the repository structure. This is not a code logic error that could be introduced by the PR's changes.
-
Pre-existing repository issue: The missing directory represents a pre-existing problem in the repository environment. The similar task failure outputs from the master branch were empty, suggesting this may be an intermittent or environment-specific issue rather than something introduced by this PR.
-
Direct inspection of changes: Reviewing the complete diff shows:
- Creation of
packages/nx/src/utils/spinner.tswith a new SpinnerManager class - Updates to various files to use
globalSpinnerinstead of local ora instances - Simplification of
newProject()calls in e2e tests - Documentation updates to markdown files (adding
{% frame="none" %}attributes and removing manual dependency management sections) - ESLint generator changes to remove deprecated TypeScript ESLint packages
- Creation of
None of these modifications would cause or relate to a missing blog directory in the nx-dev public folder.
The failure is definitively classified as 'environment_state' because it represents a missing file system resource that is completely unrelated to the PR's scope of work around spinner handling and test improvements.
A code change would likely not resolve this issue, so no action was taken.
🎓 To learn more about Self Healing CI, please visit nx.dev
|
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
This PR creates a global spinner handler and adds the runtime information to the
convert-to-inferredmigration process.The global spinner ensures a single instance of the
oraspinner. The ora cannot run several instances in parallel, so running multiple instances causes flickering due to message deletion.The
covert-to-inferredplugin migration will now show the loading spinner and progress indicator specifying how many projects have been converted.Additional changes:
Current Behavior
Expected Behavior
Related Issue(s)
Fixes #