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 option for the theme listener to ignore paths specified by the user #280

Closed

Conversation

jakebellacera
Copy link

@jakebellacera jakebellacera commented Mar 20, 2019

✌️

/cc @zendesk/vegemite

Description

The theme listener watches all files in the directory, so when working with a project with a large node_modules for instance, this can cause the watcher to perform very slowly. Another issue this solves is that multiple instances of guard can co-exist within the working directory in case you have gulp/webpack watching the src folder, for example.

How it works

Users can specify an --ignore option to have the listener explicitly ignore an array of paths. Example:

zat theme preview --ignore node_modules src styles

@jakebellacera
Copy link
Author

👋 Friendly bump on this. Would you have a moment to check this out?

/cc @ocke @sarmadsangi (and whoever else owns this project)

@jakebellacera
Copy link
Author

jakebellacera commented Apr 25, 2019

Bumping again.


Can we update the PR instructions to make sure that outside users can successfully contribute to this open source project? Right now there isn't a way to get the maintainers' attention (see: #281). I would like to get this merged or at least considered and not have to continue to support my fork as a dependency on our end.

@ocke
Copy link
Contributor

ocke commented Apr 29, 2019

Hey @jakebellacera. Sorry for the last response and thanks for the contribution. Do you think we should make zat always ignore node_modules. In that case we don't even need to make it a option. Can there be any reason zat should not ignore node_modules?

@jakebellacera
Copy link
Author

jakebellacera commented Apr 29, 2019

@ocke no worries!

A default list with node_modules would be great since the folder can cause issues with Guard. I still think the option to configure a manual ignore list will be useful for flexibility!

@hoangtrvu
Copy link
Contributor

Hi @jakebellacera, we force updated master branch so can you please rebase your PR? Thank you 🙇

@sarmadsangi
Copy link
Contributor

@jakebellacera sorry for missing out on this PR. This is great 🎉

We have seen similar issues with zat server as well, so i was thinking if we can tweak this current approach to be a bit more flexible.

Suggested solution

  1. We have a .zatignore with node_modules as default value.
  2. Read ignore paths from this file for zat theme preview and zat server

What do you think?

@jakebellacera jakebellacera force-pushed the theme-watch-ignore-option branch from 74d23a6 to e93d1dc Compare May 10, 2019 20:01
@jakebellacera
Copy link
Author

@hoangtrvu updated!

@sarmadsangi the .zatignore option would be great!

@keitheous keitheous force-pushed the master branch 2 times, most recently from 797ccf0 to 79180e0 Compare August 8, 2019 02:15
@dangayle
Copy link

This would solve my current issue. If you have too many files in your node_modules, Zat will fail with "too many open files" error.

Screen Shot 2019-12-30 at 11 57 44 AM

@sarmadsangi
Copy link
Contributor

@augustocravosilva can we get a review and approval from your team for this change?

@pezholio
Copy link

This would be really good to get in. At the moment I'm blocked from being able to manage my dependencies using Yarn/NPM

@jakebellacera
Copy link
Author

Bump. Any updates?

@jakebellacera
Copy link
Author

I'm deciding to close this PR since it doesn't look like it's going to get through. If plans change, I'm happy to reopen.

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.

6 participants