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

Fix template assertion when opening a session. #16272

Merged

Conversation

tgxworld
Copy link
Contributor

@_partials.clear
@_templates.clear
@_layouts.clear
@_files.clear
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clearing all the Hash instead of creating new ones since the subscribers are registered with the Hash objects created during setup. The previous change introduced a regression because we are actually creating new Hash objects in the duplicated context which are not the same as those registered with the subscribers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@senny
Copy link
Member

senny commented Jul 24, 2014

@tgxworld can we add those ivars into an array ant iterate over that? This way both places will stay in sync.

@@ -12,8 +12,13 @@ module TemplateAssertions
teardown :teardown_subscriptions
end

RENDER_TEMPLATE_INSTANCE_VARIABLES = %w{partials templates layouts files}.freeze
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@senny Instead of creating a new instance variable for the array, I decided to use a constant instead. Any thoughts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:+1;

@tgxworld
Copy link
Contributor Author

@senny Updated 😄

matthewd added a commit that referenced this pull request Jul 25, 2014
…pening_a_session

Fix template assertion when opening a session.
@matthewd matthewd merged commit 75eddfb into rails:master Jul 25, 2014
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.

None yet

3 participants