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

Changes In Rails 6 ActionView::Template Breaks EmptyTemplateHandler #2086

Closed
erich-team opened this issue Feb 26, 2019 · 8 comments

Comments

@erich-team
Copy link

commented Feb 26, 2019

Changes to the Template handler calls in Rails 6.0 beta breaks EmptyTemplateResolver

Specifically this change which changes actionview/lib/action_view/template.rb, line 307, from this...

code = @handler.call(LegacyTemplate.new(self, source))

... to this ...

code = @handler.call(self, source)

... breaks rspec-rails, lib/rspec/rails/view_rendering.rb, line 105

def self.call(_template)

with the following error;

ActionView::Template::Error: wrong number of arguments (given 2, expected 1)

Ruby version: ruby 2.5.1p57
Rails version: 6.0.0.beta2
Rspec version: 3.8.2

@JonRowe

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

Any PR for this should target 4-0-dev

@jamesjefferies

This comment has been minimized.

Copy link

commented Feb 27, 2019

Hi @JonRowe - targeting 4-0-dev (which currently has some test failures around Notifications) sounds like a good plan - but this patch will need to sniff which version of ActionView it's running against right? Is there a simple example of where that is already being done?

@benoittgt

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

@jamesjefferies Why not be Rails version specific? For example

if ::Rails::VERSION::STRING >= '5.1'

@JonRowe

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

TBH I'm not sure we need to gate this, our method defines one argument, it needs two on this version of Rails, seems like simply adding a second optional argument would fix it, and gate the spec instead.

(Also 👋 @jamesjefferies)

@jamesjefferies

This comment has been minimized.

Copy link

commented Feb 27, 2019

@JonRowe ah yes, that sounds like the best way to sort this .. and yes! 👋

@erich-team

This comment has been minimized.

Copy link
Author

commented Feb 27, 2019

@JonRowe I toyed with that in a monkey-patch

def self.call(_template, source)

, but given the method doesn't use _template as it is, it seemed a reasonable future-proof solution, Just a thought.

@JonRowe

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

It'd need to be _source = nil in order to remain backwards compatible.

@JonRowe

This comment has been minimized.

Copy link
Member

commented Mar 1, 2019

Fixed in #2089

@JonRowe JonRowe closed this Mar 1, 2019

siegfault added a commit to siegfault/expiration that referenced this issue May 18, 2019
Fix specs.
Need to bump to beta version of rspec for the fix for
rspec/rspec-rails#2086.
@aldesantis aldesantis referenced this issue Aug 12, 2019
7 of 7 tasks complete
megantiu added a commit to megantiu/upgrade-rails-workshop that referenced this issue Aug 13, 2019
megantiu added a commit to megantiu/upgrade-rails-workshop that referenced this issue Aug 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.