-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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: fuels dev
cleanup not killing node
#3038
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -233,7 +233,6 @@ export const launchNode = async ({ | |||
} | |||
childState.isDead = true; | |||
|
|||
removeSideffects(); |
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.
Removing this line caused the issue to go away.
I suspect that the root cause of the problem is somewhere in the fuels
package because this line didn't cause problems to e.g. launchTestNode
. It might be in the interplay between file watchers registered in the dev
command and this cleanup
function removing the files they're watching, but I couldn't pinpoint the actual root cause. I tried deleting lines I think might be causing it while having this line still on, but to no avail. We can search for the root cause in another issue that's of lesser priority.
The cleanup still happens because of the error
and exit
event listeners registered on the child above.
CodSpeed Performance ReportMerging #3038 will not alter performanceComparing Summary
|
c5aab56
to
c187c5e
Compare
…values Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
shell: bash | ||
run: | | ||
echo "$PWD/internal/forc/forc-binaries" >> $GITHUB_PATH | ||
echo "$PWD/internal/fuel-core/fuel-core-binaries" >> $GITHUB_PATH |
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.
I added these to the path so that the spawn can run from the cwd
of the temporary test project instead of running pnpm fuels init --path ${paths.init}
from our repo's root. Both of them are using the fuels-forc
under the hood, but it feels more correct to have all the commands be running from inside the temporary test folder, pnpm fuels init
included.
As an alternative, I could initiate the temporary project and pass in the --fuel-core-path
and --forc-path
arguments to point to these binaries. I opted for adding forc
and fuel-core
to the path like this because this wasn't the first time I had to debug this problem only to come to the conclusion that forc
and fuel-core
aren't in the path.
Coverage Report:
Changed Files:
|
fuels dev
withCTRL+C
(SIGINT
) doesn't killfuel-core
node #2889Checklist