fix: ensure paths exist before trying to watch them #17
Conversation
There was a problem hiding this comment.
Good patch @adrianthedev. Requesting a couple of changes, but good call filtering only existing dirs 👍
lib/hotwire/spark/engine.rb
Outdated
| @@ -8,7 +8,7 @@ class Engine < ::Rails::Engine | |||
| config.hotwire.spark = ActiveSupport::OrderedOptions.new | |||
| config.hotwire.spark.merge! \ | |||
| enabled: Rails.env.development?, | |||
| css_paths: File.directory?("app/assets/builds") ? %w[ app/assets/builds ] : %w[ app/assets/stylesheets ], | |||
| css_paths: %w[ app/assets/builds app/assets/stylesheets ], | |||
There was a problem hiding this comment.
I'm afraid we can't get rid of this to avoid the duplicated signal problem mentioned here #11
There was a problem hiding this comment.
Oh, I see.
Reverted this change.
lib/hotwire/spark/file_watcher.rb
Outdated
| ensure_paths_present @callbacks_by_path.keys | ||
| end | ||
|
|
||
| def ensure_paths_present(paths) |
There was a problem hiding this comment.
Would you mind calling this only_existing_paths. This is an in-house convention, but we usually use ensure to name methods that may raise or take other action, not those that purely return values.
There was a problem hiding this comment.
Yup. Makes sense! That's a better name.
|
Thanks for the review, Jorge 🙌 I was talking to Paul about some of the design decisions you took with this gem. |
|
Thanks @adrianthedev! I added that because there was more to coordinate than in a regular engine, so it's nice to have a dedicated place to orcherstate things. Also, I pushed this minor tweak to main. |
|
Love it! |
Some apps might not have the directories that are set by default.
This PR checks if the path exists before trying to watch it.
I ran into this while trying to run it in an engine. The
dummyapp doesn't have theapp/assets/stylesheetsandapp/javascript/controllerspaths.This is one example. Others might structure their application differently as well.
Note
As I am still new to contributing here, I am unfamiliar with some of the design patterns and decisions made thus far in this repo. If I broke any patterns you've established so far, let me know and I'll address that.