Revert "Avoid loading ActionView::Base during initialization (#1528)"

[jonrohan]

What are you trying to accomplish?

Reverting #1528

I went to update view_component from 2.74.0 to 2.81.0 the sorbet checks for Primer View Components in GitHub.com's codebase and it ended up removing a lot of the definitions from the rbi file.

What approach did you choose and why?

To find what caused it, I rolled to each version until I found one that didn't break, we were on 2.74.0 in dotcom and it broke after upgrading to 2.75.0.

Anything you want to highlight for special attention from reviewers?

[Spone]

@jonrohan please also see #1571 (comment)

[camertron]

@Spone ah yeah thanks for that 😄 We discussed reverting the PR during office hours today as well.

[bradparker]

Hi there 👋. The PR this reverts got us out of a sticky situation. Now that it's reverted, we're stuck again and are unable to upgrade from 2.81 to 2.82. It's a bit of a funny one and involves a dependency doing something a little unusual. I'll try and explain in case it's helpful.

Here's what happens for us.

• We use JBuilder.
• JBuilder monkey-patches the ActionController::ApiRendering module to include ActionView::Rendering on initialisation (rails/jbuilder@26b32cc). It does this to enable to use of .jbuilder templates from within the views directory for API controllers (ActionController::API), as if rendering HTML-generating templates in an non-API controller. It reuses the non-API view-rendering machinery.
• Prior to the change introduced in Avoid loading ActionView::Base during initialization #1528 this monkey patching occurs after ViewComponent::Base is evaluated.
• When that happens, for whatever reason (and I haven't been able to follow this all the way through), the un-monkey-patched state of ActionController::ApiRendering has already been included in ActionController::API, the monkey patch doesn't do anything, it's too late.

The result is render_to_body ends up falling all the way through to the default case: https://github.com/rails/rails/blob/08b3cc3d3254d92647a0812a9c9b6f8b22375ac6/actionpack/lib/abstract_controller/rendering.rb#L51-L52. And we get an empty string where we expected a JSON document rendered via a jbuilder template.

Am I making sense? Have I missed something? Is there an alternative we should be pursuing do you think? Is there another way JBuilder could be achieving what they want?

Any and all help is greatly appreciated :)

[camertron]

Hey @bradparker, thanks for bringing this up! As @jonrohan mentioned above, we reverted #1528 because it caused a bunch of Sorbet .rbi files to disappear when we tried to update ViewComponent in github.com's monorepo 😱 We tracked the breakage down to this PR and sure enough, reverting it fixed the problem. Since dotcom doesn't suffer from the problem #1528 was trying to solve, we didn't hesitate to revert it. Loading ActionView::Base too early sounds like something we should fix for a bunch of reasons though. Let me dig in today or tomorrow and see if I can figure out the Sorbet issue.

[camertron]

Alright, so interestingly the Sorbet issue appears to have gone away, i.e. I couldn't reproduce it again. All dotcom tests pass too, so I'd say let's revert the revert. Here's the PR: #1659