Make it compatible with Ruby's BasicObject #21
Conversation
| scope.instance_exec(binding) { | ||
| @_jbuilder_view_path = view_path | ||
| @_jbuilder_locals = locals | ||
| } |
There was a problem hiding this comment.
What's the purpose of this change? Is this for BasicObject compatibility or an unrelated changed?
There was a problem hiding this comment.
It's related because BasicObject doesn't respond to #send.
There was a problem hiding this comment.
Semi-related: I removed the line that was setting data because it wasn't clear to me where data was previously instantiated, and because the tests were still passing without it.
There was a problem hiding this comment.
Unless you feel like you have a good reason not to would you mind adding data back in? Removing it seems unrelated to this PR and I'd rather not risk introducing a bug.
There was a problem hiding this comment.
@anthonator I see your concern and totally agree with it.
But if add that data back as third line, it raises a NoMethodError. Which leads us to the initial issue: I don't understand where is defined. 😉
|
@jodosha thanks for submitting this! Do you know if the other Tilt templates use |
|
@anthonator My pleasure to make gems to work together. 😉 I don't know if other Tilt templates use The suggestion to use |
|
Other than an issue I brought up above I think this looks good. Would you mind adding some information to the README on how to use |
|
@anthonator sure thing. 😄 |
Hi, I'm the author of Hanami, the web framework for Ruby.
Our gem
hanami-viewusestiltto render templates. During the rendering process, the view passes to theTilt::Templatea scope which inherits from Ruby'sBasicObject.This decision was taken to avoid conflicts with methods defined by Ruby's
Object, which had the result of shadowing methods in the scope. This is one example of the clash: hanami/view#28There are two devs who reported a bug both in your repo and in ours as well, because our gems aren't compatible.
This PR aims to fix these problems, by making
tilt-jbuildercompatible withBasicObject, and by consequence with Hanami.Please consider to merge this, I believe that
tilt-jbuildercan benefit from this, regardless the Hanami compatibility. Again, see hanami/view#28 for a concrete example.Thanks in advance for your time.
Fix: #20