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 ability to disable watching on local dev registry #595

Merged
merged 1 commit into from
Aug 12, 2017

Conversation

badsyntax
Copy link
Contributor

@badsyntax badsyntax commented Aug 5, 2017

For our use case, we run visuals tests against the local dev registry. This process saves screenshots to disk which then kicks off an OC package task and sometimes this errors out whilst the visual tests are running. We'd love to be able to disable watching for this scenario.

Would need to update: https://github.com/opentable/oc/wiki/Cli

@badsyntax
Copy link
Contributor Author

Actually it looks like --hotReloading=false is good enough, so i'm gonna close this one.

@badsyntax badsyntax closed this Aug 5, 2017
@badsyntax badsyntax reopened this Aug 8, 2017
@nickbalestra
Copy link
Contributor

@badsyntax I see you reopened this PR, any more context to share?

@badsyntax
Copy link
Contributor Author

badsyntax commented Aug 10, 2017

Hi @nickbalestra

--hotReloading=false gets us most of the way but i still feel it's a workaround.

For our use case we simply don't want any watching at all. Even with --hotReloading=false, the watcher still monitors every file (including node_modules) and this can be a pretty intensive task. We want to run a dev registry, with production built files, and then perform some tasks on that registry (like do visual regression tests). We have no need for watching, and it eats up CPU and disk unnecessarily.

In some cases it results in obscure errors, like ESOCKETTIMEDOUT when doing network requests. We think this is related to open file limits (see "Open File Limits" here: http://kb.imakewebsites.ca/2014/01/03/nodejs-setup-tips-for-devops/), but we're not sure. Multiple people in our team have had this error, and sure, this is more of a generic problem rather than a problem with OC, but it would be still be useful to be able to disable this behaviour in OC.

@ryaanwells
Copy link
Contributor

Seconding @badsyntax on this one 👍

Just to add one more case this would be useful in - consider when the OC component loads some ES6 JS, scss or React code that needs transpiling - you need to rebuild entirely to see the changes (hot reloading the dev registry won't help). I want to control the lifecycle of the registry reload as part of my component rebuild lifecycle and would rather it didn't do anything in isolation.

Copy link
Contributor

@nickbalestra nickbalestra left a comment

Choose a reason for hiding this comment

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

👍 Thanks for this @badsyntax !

@nickbalestra nickbalestra merged commit 4cffd5c into opencomponents:master Aug 12, 2017
@nickbalestra
Copy link
Contributor

@badsyntax published with v0.40.6, and added documentation

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.

3 participants