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

[WIP] Pass context to renderer #152

Closed
wants to merge 1 commit into from
Closed

Conversation

Borzik
Copy link

@Borzik Borzik commented May 3, 2016

This PR is based on a fix from #150

It adds passing context to renderer, so we can call image_path, asset_path, and other helpers again.

@AlexRiedler
Copy link
Collaborator

I think this makes sense; is there docs about this ? cause this only affects Sprockets 4+ right ? or am I mistaken ?

@Borzik Borzik changed the title Pass context to renderer [WIP] Pass context to renderer May 5, 2016
@Borzik
Copy link
Author

Borzik commented May 5, 2016

Sorry for such a small description, my main purpose was to get someone's attention to that fix and issue itself (I've faced it). I thought that since this issue was fixed for me this way, it should work for everyone, but now I realize the problem is deeper.

I am using Sprockets v 3.6, not Sprockets 4. But for some reason HandlebarsProcessor is being used for rendering, not HandlebarsTemplate class, although Sprockets 3 HandlebarsTemplate is installed.

For more information, please check this reduced test case. I've tried to do it as small as possible, only one template, single view, no dependencies. All versions (both working and failing) are added to Gemfile, so you can see what works for which versions (by commenting/uncommenting some parts of code).

@Borzik
Copy link
Author

Borzik commented May 5, 2016

Actually the problem is much simpler: We are testing that Sprockets version is less than 3, but should test if it's less than 4. Created another PR for that.

@Borzik Borzik closed this May 5, 2016
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.

2 participants