Skip to content
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(project): queue spinner promises on watch #20

Merged
merged 1 commit into from
Jun 8, 2017

Conversation

acburdine
Copy link
Member

fixes #19

  • on fast rebuilds, the change event would be fired quickly enough that
    it would cause the spinner to succeed before it could be started
    completely. This ensures the succeed call happens after the start call.
    It also fixes the timer

Copy link
Contributor

@davewasmer davewasmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof this is a bummer, but yea, I see the race condition you are describing, and this change is unfortunately necessary for this one spot since it's all event based.

I don't see a cleaner way of doing this off the top of my head. One small tweak I'd suggest though - could we maybe call it something like spinnerStart or something? Just so it's clear that it's purely there to track spinner state, not actual build/watch state.

fixes denali-js#19
- on fast rebuilds, the change event would be fired quickly enough that
it would cause the spinner to succeed before it could be started
completely. This ensures the succeed call happens after the start call.
It also fixes the timer
@acburdine
Copy link
Member Author

Fixed 👍

@davewasmer davewasmer merged commit 8ab9102 into denali-js:master Jun 8, 2017
@acburdine acburdine deleted the yay-race-conditions branch June 13, 2017 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spinner throws an error and exits process on rebuild
2 participants