trailblazer / cells

View components for Ruby and Rails.
https://trailblazer.to/2.1/docs/cells.html
3.07k stars 236 forks source link

Cells 4 API reconsider name change of render_cell helper #262

Closed nacengineer closed 9 years ago

nacengineer commented 9 years ago

@apotonick First, I must say that I really love this gem. Thanks for all the hard work! It makes complex views SO much simpler and I believe it should be in every rails devs toolbelt.

One issue/question I wanted to raise was that I was looking at v4 and I am hoping you'll reconsider renaming the render_cell helper to cell.

I understand SemVer allows for these major API changes but this is a trend in the ruby community that needs to be somewhat pushed back against IMO, e.g. rspec changing spec_helper.rb to rails_helper.rb. Why? Sure it adds more clarity sense but was it really necessary?

This is a breaking change that will affect A LOT of LOC in order to support the new helper. As you probably already know, most IDEs that rails devs use aren't the greatest at this type of refactor and lets be honest our tests/specs have enough side effects and missing coverage already that this change could cause more side effects to an upgrader.

From the gem's POV its cleaner (8 chars) and perhaps breaks the mental (and real) association of ActionView render and cells uh render but from the gem user's POV this is THE API/DSL call. This change creates the need to touch every instance where the cell is drawn. I feel it has the possibility of causing a few side effects which could hinder the quick take up of the new version and require you to support the old one longer.

While its not my gem and the reasons for this change are surely sound, I'm guessing its the ActionView separation that prompted it. I just wanted to put a little plug in for no change. :)

seuros commented 9 years ago

You can alias it via a monkey patch if you want, officially version 3 api won't change to not cause the failing tests apocalypse.

apotonick commented 9 years ago

Hey David, thanks for that valid comment. The thing is, I completely change the semantic of a cell. In Cells 4 we get away from Rails and its controller-orientation to an actual view model.

Here's the difference.

render_cell(:song, :show, @song)

results in

class SongCell < Cell::Rails
  def show(song)

The song argument is passed into #show in the old dialect.

Now, Cells 4.

cell(:song, @song).(:show)

In Cells 4, the cell helper instantiates a cell with all the arguments, then calls the view state*.

class SongCell < Cell::ViewModel
  def initialize(song)

  def show

As you can see, you'd have to change your view states anyway (unless you patch).

You can surely introduce your own #render_cell helper that does exactly this behind the scenes (you can still do cell(:song).show(@song) but I have to warn you: The way helpers work has changed, some ActionView hacks are not gonna work in Cells 4, and many other things, such as a changed #render API.

Cells 4 is not meant to be a drop-in replacement for your cells 3 components - you have to migrate them manually in some cases. I can totally understand your concern and we will make this as painless as possible, but in order to smash this milestone we have to be aggressively changing, otherwise we will end up like other major "MVC" frameworks and keep treading water. :wink:

*BTW, the call styles are documented in my book in chapter 6 - I am gonna publish this chapter this week.

apotonick commented 9 years ago

The solution might be to provide a render_cell compatibility helper.

def render_cell(name, state, *args)
  cell(name).(state, *args)
end

Done. Hahaha, not sure if that works for each and everyone, though. :laughing:

nacengineer commented 9 years ago

I can buy into that. Glad you put a lot of thought into it.

Knowing your team, I assume you'll keep the 3 branch from dying for a reasonable time which is my main concern. i.e. rails 2 fixes fairly quickly become upgrade to rails 3 which was a massive API change and required community built tools like rails upgrade, etc. Guess I'm still a little gun shy from that change and all the crazy little OCD changes devs tend to make.

Like I said, I'm really happy with the gem and looking forward to it being even faster! :) Keep up the great work. I relent and will close this issue. :)

apotonick commented 9 years ago

I am glad you came up with that problem. Sometimes I am too focused on my "innovation trip" and forget about all the poor users who have to deal with my attitude. :laughing:

We added the cells-3 branch so we can keep maintaining it. I just had a funny idea to allow both gem versions in one app (basically not a problem) but then we have to rename cells-3 to some other gem name.

barttenbrinke commented 9 years ago

Wow, this change is really big for us. I am kind of stuck as how to begin. Currently we mix in extra methods in Rails::Cell, but I guess I can safely move those to Cell::Viewmodel? And then rewrite all of our 25+ cells. And one thing I was missing in the migration guide was the fact that call has been removed. Could you tell a bit more about the old and the new behaviour?

nacengineer commented 9 years ago

@barttenbrinke like all rails apps, for my upgrades I'm using a feature branch in my git repo and making sure all the cells and views where the cells are used have specs. Without the specs/tests I think the upgrade is infeasible as you won't know what's broken until its in production. One thing I would recommend is using the "render_views" helper in your Controller specs.

Having said that I agree this is a considerable API change in order to shed the bloat of ActiveRecord but this is the joy of Dynamic Imperative programming (insert sarcastic smiley here) and is one of the reasons I'm transitioning to Haskell ASAP

apotonick commented 9 years ago

@nacengineer Haha. Ehm, not sure how Haskell will save you from API changes, though? If you change the signature of a function in Haskell, you have to change your user code, too, don't you? Disabuse me as I haven't programmed Haskell in a long time. :hourglass_flowing_sand:

@barttenbrinke Sorry to increase your workload. I had to change the public semantics - we needed to get rid of this "Rails crust" to keep innovating. The result is a super-lean core library now, some users reported rendering speed-ups of factor 2-3. :rocket:

#call hasn't been removed - what makes you think so? Do you have specs for all your cells? If not, this is a good chance to write quick smoke tests, with Cells 4 it's even simpler now without all those hacks to make AV behave the way we want it in tests.

I will try to add another section to the upgrade guide to address your trouble, @barttenbrinke. Also, make sure to join the IRC (Freenode) #trailblazer channel! :beers:

nacengineer commented 9 years ago

@apotonick not saying it'll eliminate API changes but IMO the Haskell community is less prone to make changes without a lot of consideration toward legacy code bases. For example if you pick up the Real World Haskell book from 2010 it still applies to most of the current ghc version. If you try the same with a ruby book your results will vary. IMO Functional Programming requires a lot less upkeep over time. I'm not against a server that can compile to a single bin (snap) and can handle requests in the 60k+ range vs rails 2-6k range. Plus concurrency.

I see this upkeep issue as a massive problem/viewpoint in the ruby/rails community, eg. rails 2 -> 3 was a massive undertaking and some companies just couldn't manage it. rspec 2 -> 3 was a pretty major API change about half of my code is still stuck on rspec 2. rails 4.0 -> 4.1 -> 4.2 pretty much within a year of each other with some serious minor API changes that deprecate current functionality but add very little IMO improvement wise.

So the end result is a developer spends a majority of time just jumping their code through API changes vs. adding features to their applications. It becomes quite frustrating. Now we have a major release (5.0) about to break and its going to require more halt your development and update your code bases or be left behind security wise.

Another example is ruby 2.0.0 -> 2.1 -> 2.2. ruby 2.0 is pretty stable and there is a known memory leak in 2.1 which allegedly was fixed in 2.2 but not in 2.1?!? However I still can't get puma to not bloat memory in 2.2 and have to run it in 2.0. Yet ruby 2.0.0 is in security maintenance phase and heading towards end of life while 2.1 and 2.2 are still kind of iffy IMO and probably deserve a Beta tag. Also things like refinements. What the hell is that all about? The community wasn't clamoring for that feature and most outright refuse to use it.

Generally I think in our community we too easily leave old code bases behind from a security standpoint and justify it with "its better". Again these are probably more the ramblings of a burnt out developer who has had to maintain some nasty legacy codebases but I'd like us to have fewer API changes and work on some of ruby/rails core issues, i.e. more changes like ActiveModel/ActiveRecord separation or why was rails 2 faster performance wise than rails 3, etc.

Anyway I shouldn't rant anymore here as this isn't the forum. LOL

Thanks @apotonick and team for all the work you do on this gem. I think its a great gem and helps me make my Rails app move toward more of a "functional" paradigm. :+1:

apotonick commented 9 years ago

Haha, this totally is a forum and you don't rant but bring up valid points.

Please don't use Rails as an example for change. Rails hasn't changed at all in the last decade, it's still stuck in it's primitive "MVC" thinking where other communities have moved on a long time ago. Rails problems actually is the obsession to be backward-compatible. This is why (what you call) "API change" in Rails means that AR scopes suddenly have a different naming scheme or callbacks can be stopped by emiting a special symbol.

This is not change, this is a cosmetic improvement. Still inconvenient, as you say, but superficial and absolutely not fixing the actual problems Rails has on an architectural level.

Anyway, to come back to what you say. I don't think it's wrong to innovate. Innovation in programming means new structures, new objects (or functions) and new flows. And this means work for upgraders. :grin: I can't find this in Rails, though, whether this is a good or bad thing, I won't say in public haha.

Think about these changes as this: I aggressively change my gems in order to make your life better. I do not simply decide that this method signature sucks or that class is useless, I collect input over the years and play with ideas permanently.

At some point, you simply figure out your original idea was suboptimal or misleaded (e.g. by trying to be "Rails-like"). And then you have to hardcore-change. I myself had to change about 20 cells in our current production project, it didn't really hurt and in the end I could see a significant performance increase, zero hacking in tests anymore, and the good feeling of having decoupled my code from Rails.

nacengineer commented 9 years ago

@apotonick agreed re:rails. I think my issue is with gems and code that doesn't follow your approach. Obviously its insane to think there will be no change. I think perhaps I'm misspeaking when I say API changes too. Really I mean OCD changes made in the spirit of the new and not always thought out shiny. Too often I see changes such as changing a method name arbitrarily or refactorings that seem to only be for the purpose of satisfying a developers' fancy or implement some new feature that hasn't really been proven, e.g. refinements in ruby. But our community does this a lot and justifies breaking changes with SemVer.

If more developers took your approach the community would be better off. My main issue is that ruby is so dynamic and can so easily have side effects that to make changes without thought/consideration is a dangerous trend. If we had type safety it might be different, but that's never going to happen in ruby. Although gems like Contracts gives me some hope there can be intermediate solutions for ruby.

Anyway as I said I have no problem with the changes you made here. I think the benefits outweigh the entropy cost plus you are moving in a super sane direction by distancing yourself from ActiveRecord and toward SRP type of design in cells. :+1:

apotonick commented 9 years ago

So true! One thing that really scared me off was when Rails suddenly decided to drop old Ruby support, mainly to use Refinements, which are feature I am never ever gonna use (ever!). Honestly, a web framework that uses super low-level language tools, that's just not good. Another thing was when everyone dropped 1.8 support to have 1.9 hash syntax... WTF?

I would love to see typing in Ruby, and different method signatures depending on the argument types, as we have it in Java. Thanks for your kind words! :heart: