Configure Cache Sweeper to Also Run in "test" Environment

[rience]

Cache sweeper is not being used in "test" environment.

When using dartsass-rails - we're running dartsass:build before kicking off RSpec tests. However, we're doing that when RSpec is initializing - so after Propshaft is initialized. When dartsass creates files in assets/builds - they're not picked up by Propshaft because cache sweeper is inactive in test.

Looking at file load_path I can see this documentation

# Returns a file watcher object configured to clear the cache of the load_path
# when the directories passed during its initialization have changes. This is used in development
# and test to ensure the map caches are reset when javascript files are changed.
def cache_sweeper
end

Which suggests that cache sweeper should be running in test environment. So this PR is updating railtie.rb and sets cache sweeper to run in test by default.

[brenogazzola]

That comment is outdated. I disabled the sweeper in test because it slows down the suit of integration/system tests by quite a bit (from 5 to 7 minutes in our case if I remember)

Propshaft loading before rspec shouldn’t be a problem since the loadpath only gets memoized on the first request. So there is something else going on here.

[rience]

You're totally right - I can see now where the problem was. So app/assets/builds directory didn't exist when RSpec was starting. And when framework starts Propshaft in railtie.rb is only accepting directories that do exist.

app.config.assets.paths.unshift(*paths["app/assets"].existent_directories)

Of course I did fix that by adding .keep file into that directory and push that into repo. However, do you think it should stay like this - or could I work on PR to change that behaviour?

[brenogazzola]

I think it should stay disabled by default, since it works fine and is faster.

However if there is a use case for a config option to enable it, that cannot be solved in another way (like ensuring the build folder is committed) and enough interest from others (which my own PR #57 does not have), I’d welcome a PR.

[rience]

I totally get you - yes, that should stay disabled.

My question was rather about - not filtering out these directories that don't exist during initialisation phase.

[brenogazzola]

I'm still a bit confused about your question. The line you pointed is not filtering out directories that do not exist. Instead, its telling propshaft that all folders named app/assets (in your app and the gems in the Gemfile) should be added to the load path.

If a new directory (like build) is created inside app/assets after that line is run, but before the first request or the precompile step, propshaft will still pick it up. That's because searching for directories/files inside app/assets is delayed to the lost possible moment.

So:
1 - You run assets:precompile
2 - Propshaft loads, and app/assets is added to the paths array;
3 - Files and directories are created inside app/assets by the dartsass (this is done by enhancing the precompile task)
4 - Propshaft goes through every path in the paths array, reading every directory and file there
5 - All files/directories found are compiled and copied to the output path.

If I'm not mistaken, the problem with builds not existing is that the first time dartscss runs, it "crashes" and ends up creating the directory, but not the files inside the directory. The second time it runs the directory is there, so it manages to add the files and propshaft picks them up. Of course, if you are precompiling, then that step only happens once, and you end up without the files the dart should have created.

[rience]

Slightly different scenario

1. You start an app (I've pushed an example one: https://github.com/rience/propshaft_issue)
2. You open browser and open http://localhost:3000 (so that Propshaft is initialisated) - you should see an error that application.css can't be found (that's correct)
3. You then in Terminal run bundle exec rails dartsass:build (this will create app/assets/builds directory but also transpile app/assets/builds/application.css)
4. You refresh browser

Result: Transpiled application.css will not be picked up until you restart server.

[brenogazzola]

Ah, ok. I get it. Since builds did not exist it was not added to the file watcher, so it does not matter how many times you refresh, the sweeper will not trigger. Thanks the example app made it easier to understand.

My initial reaction is to say: just make sure the builds folder exist, no changes needed. But if you have a solution that does not overly complicate the code or has a performance impact, I'd welcome a PR for it.

[rience]

Ah, ok. I get it. Since builds did not exist it was not added to the file watcher, so it does not matter how many times you refresh, the sweeper will not trigger. Thanks the example app made it easier to understand.

My initial reaction is to say: just make sure the builds folder exist, no changes needed. But if you have a solution that does not overly complicate the code or has a performance impact, I'd welcome a PR for it.

That's a tricky thing to implement. You can't add file watcher on something that does not exist.

What about adding warning similar to this that you get when you precompile assets: "Warning: You are precompiling assets in development. Rails will not serve any changed assets until you delete public/assets/.manifest.json"?

Let's say something like this:

Warning: Following asset paths are configured but they don't exist
  app/assets/builds
  xxx/yyy/zzz

[brenogazzola]

I'm seriously considering having propshaft just create the builds folder during boot. There are enough people having trouble with this that we can probably argue that it's propshaft's responsibility to do it, since "improve integration with bundlers" is one of its objectives.

[rience]

Shouldn't this rather part of Rails project template? All these solutions - jsbundling, cssbundling-rails and dartsass-rails are using assets/builds library in default configuration. So having this in one plane i.e. rails might be the best option?

[brenogazzola]

The builds folder is an implementation detail for the asset pipeline to better integrate with bundlers, so it doesn’t feel right that rails should know about it?

But propshaft is the asset pipeline, and having it create the builds folder during boot provides the single plane as you mentioned. Also, the entire problem here is that people keep deleting said folder, so just creating it once (in the template) wouldn’t be enough.

[dhh]

Seems to me that the bundler plugins should be adding app/javascript/builds/.keep when they're setup?

[dhh]

Either way, this isn't the fix.