-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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(rollup): use copySync
instead of async copy
#4001
Conversation
`yarn playground:new` was generating random errors related to unlinking/deleting files that did not exist. File copying was using an asynchronous method to move files into playground directories. This change makes the copy/move operation synchronous and eliminates race conditions, ensuring `yarn playground:new` always succeeds.
|
Hi @n8agrin, Welcome, and thank you for contributing to Remix! Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once. You may review the CLA and sign it by adding your name to contributors.yml. Once the CLA is signed, the If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at hello@remix.run. Thanks! - The Remix team |
Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳 |
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 don't think this changes anything tbh, as we're awaiting for the sync to be finished before we continue (just like we would use the synchronous function) 🤔
copySync
instead of async copy
@MichaelDeBoey regarding this not changing anything, a trivial repro is to checkout a new clone of remix and run
I agree that in theory the |
@MichaelDeBoey I said to put this PR up. It's something I've ran into locally quite a bit and I address it with the above solution. I'm honestly not sure why async ops are breaking here, but they are. |
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.
Seems fine 👍
yarn playground:new
was generating random errors related to unlinking/deleting files that did not exist. File copying was using an asynchronous method to move files into playground directories.This change makes the copy/move operation synchronous and eliminates race conditions, ensuring
yarn playground:new
always succeeds.