Asset dependencies are not respected in `render` plugin

[jgarth]

Hi Jeremy,

thanks for making & maintaining a really cool framework. I've found some unexpected behavior when rendering SCSS assets with dependencies.

Summary

When I change a dependent SCSS file, the rendered output does not reflect my changes.

Given this configuration

plugin :assets,
  css: {
    checkout: 'index.scss',
  },
  dependencies: {
    expand_path('assets/css/checkout/index.scss') => Dir['assets/**/*.css', 'assets/**/*.scss']
  },
  css_opts: { style: :compressed, cache: false },
  timestamp_paths: true

assets/css/checkout/index.scss:

@import 'other';

assets/css/checkout/other.scss:

<content is irrelevant>

Expected behavior

Changes to other.scss should be reflected upon page reload.

Actual behavior

Changes to other.scss are only reflected upon server restart, not before.

Probable cause

Looking at https://github.com/jeremyevans/roda/blame/master/lib/roda/plugins/render.rb#L262,
the TemplateMtimeWrapper's modified? method has no clue about the template's dependencies (i.e. other.scss), and as such always uses the mtime for the template file itself (i.e. index.scss), instead of the highest mtime of its dependencies. This leads to the template not actually being reloaded in line 270. This, in turn, means that the changes in other.scss are not reflected when rendering.

Changing the render function to always set @mtime = Time.now at the beginning of the function produces the expected behavior, but I'm guessing this "disables" template caching altogether.

I can submit a PR if you point me in the right direction for a fix.

Thanks!

[jeremyevans]

I agree this is a bug. We probably need the same type of code that the assets plugin uses, where you can pass an array of dependencies and it checks the mtime for all of them. Maybe we can pass the :dependencies option for the asset in the render call in the assets plugin, and then in render plugin, in retrieve_template, after creating the TemplateMtimeWrapper, set the dependencies on the template if render_opts[:dependencies] is set. Then in TemplateMtimeWrapper#modifed?, have it check all dependencies as well as the path itself, similar to the code in the assets plugin.

[jeremyevans]

@jgarth Is this something you plan to work on in the near future? If not, I can work on it before the next release.

[jgarth]

Yes, I'd like to submit a PR over the weekend. However, if this is more urgent to you then by all means go ahead.

[jeremyevans]

That sounds great. The weekend should be fine. The next release isn't for a couple weeks.

[jeremyevans]

I plan to work on this shortly.

[jgarth]

I added a PR with what I currently have, but I failed to incept a proper spec for the changes to the asset plugin

[jeremyevans]

OK, thank you. I was working on a spec first, so once I have a failing spec, I can hopefully merge the PR and test that it fixes the issue.