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

add delay option in case of longer builds #38

Merged
merged 2 commits into from
Oct 29, 2017

Conversation

marcelkalveram
Copy link
Contributor

Ran into the same issue as #25 where the reload had to be delayed due to longer build times.

Unfortunately, this option won't get included in tiny-lr (see mklabs/tiny-lr#110) so I decided to add it here instead.

@statianzo
Copy link
Owner

Sounds good. Can you add documentation to the readme?

@marcelkalveram
Copy link
Contributor Author

Yeah, sure. Will update the PR.

@marcelkalveram
Copy link
Contributor Author

Updated.

@statianzo
Copy link
Owner

Thanks. Just to be clear, the delay isn't to account for webpack not being complete yet, correct? If livereload-plugin is firing before webpack is done, then there's a bug.

@marcelkalveram
Copy link
Contributor Author

The delay is running after the done event has been fired by the compiler. So I'm pretty sure it runs when webpack has finished compiling.

I've tested it in my own app and was working reliably there. Feel free to test it out yourself.

Thanks!

@statianzo
Copy link
Owner

Sorry, I was unclear. Without this delay feature, livereload is being triggered before webpack has finished?

@marcelkalveram
Copy link
Contributor Author

Oh, I see what you mean. I don't think there's a Webpack bug. Live reload fires the done event after it's finished compiling.

However there are cases (like mine) where we run a Middleman-based stack, and the underlying build tool has to copy the files to another directory. While or before that happens, Live Reload has already triggered a reload. However we need to add a tiny delay to accommodate for this extra delay caused by the custom build tool.

Does that make sense?

@statianzo
Copy link
Owner

Yes. Thanks for following up and explaining. Just wanted to be sure there wasn't a bug hidden in the plugin. I appreciate the contribution!

@statianzo statianzo merged commit c5b6ae7 into statianzo:master Oct 29, 2017
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.

2 participants