komposable / komponent

An opinionated way of organizing front-end code in Ruby on Rails, based on components
http://komponent.io
MIT License
428 stars 31 forks source link

Komponent's view_renderer is leaking to Rails #144

Closed robink closed 5 years ago

robink commented 5 years ago

This commit seems to have introduce a bug in Rails 5.2 support (and certainly more versions) :

https://github.com/komposable/komponent/commit/3aed17227e1a9d89586bc61905f304fa202b74b3#diff-d662fd06dc2cbab92ab384c4af54a041L12-R12

It happens that @view_renderer == controller.view_context.view_renderer is now true, which means komponent is leaking its view_renderer manipulation to Rails.

It prevents using both komponent rendering and standard Rails rendering.

Spone commented 5 years ago

@robink can you write a failing test case for this?

robink commented 5 years ago

I think I could indeed ! Just let me a couple of days to find some time to work on it

robink commented 5 years ago

Here you go ;-)

Spone commented 5 years ago

Thanks a lot @robink for contributing these tests. They seem to pass on the Rails 6 environment! We have to investigate how to fix this.

robink commented 5 years ago

Well I guess the fix is quite easy if you look at the commit that introduced the bug (cf start of this thread). I'm ok to do it if you want (next week).

Spone commented 5 years ago

Yes I guess we have to do @view_renderer = @context.view_renderer.dup

It would be great if you had the time to try this fix and add it to your PR!

Spone commented 5 years ago

Fixed in #146