Skip to content

Removing BlankSlate and simplifying implementation #59

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

Merged
merged 11 commits into from
Sep 17, 2012

Conversation

rolftimmermans
Copy link
Contributor

Hi,

This pull request is a follow-up of #54. It is a refactoring of the implementation that achieves a couple of things at the same time, which mostly depend on each other:

Remove dependency on BlankSlate

This removes warnings about object_id being unsafe to be undefined (#56). Instead Jbuilder is now a subclass of ActiveSupport::BasicObject (for 1.8 compatibility) and loses a dependency.

Instance of Jbuilder is reused for child elements

This simplifies the implementation of Jbuilder and reduces the number of objects created. Subclasses also no longer have to implement _new_instance, which was error-prone: even the bundled JbuilderTemplate did it wrong before #54 was merged.

This improves performance marginally. Compare before/after (benchmark: https://gist.github.com/3638182 ):

             user     system      total        real
before   3.670000   0.010000   3.680000 (  3.702445)
after    3.210000   0.020000   3.230000 (  3.236420)

As a side effect, it would be possible to support the following API style:

# Old style, argument to block.
json.post do |json|
  json.title @title
end

# Possible new style, no argument necessary.
json.post do
  json.title @title
end

Of course this would be a breaking API change. So instead of yielding a child Jbuilder object self is yielded to ensure 100% API-compatibility.

Tests are fixed for Ruby 1.8

Ruby 1.8 support was implicit but never actually tested and the tests were written for 1.9. Where necessary, the tests have been adjusted to also run on 1.8.

dhh pushed a commit that referenced this pull request Sep 17, 2012
Removing BlankSlate and simplifying implementation
@dhh dhh merged commit a4e5e5d into rails:master Sep 17, 2012
@jjaffeux
Copy link
Contributor

@dhh @rolftimmermans I think this pull request broke partial! method.

I get the following exception : ActionView::Template::Error (uninitialized constant JbuilderTemplate::Hash when trying to use something like json.partial! "path/to/template", posts: @posts

dhh pushed a commit that referenced this pull request Sep 19, 2012
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.

3 participants