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

Avoid loading ActionController::API constant #529

Conversation

nvasilevski
Copy link
Contributor

@nvasilevski nvasilevski commented Feb 7, 2022

Partially related to - rails/rails#44330
Currently self == ActionController::API check causes ActionController::API to be loaded for apps that do not need it.

To avoid loading the constant I'm changing the hook to be registered on the action_controller_api so we always know that self is API constant if the hook is being called
action_controller_api name is available from Rails 5.2.0.beta1 - rails/rails@35fac87

It's fair to mention that the change isn't going to work on Rails 5 versions lower than 5.2.0, is this something we should be concerned about?

@@ -16,11 +16,9 @@ module ApiRendering
end
end

ActiveSupport.on_load :action_controller do
if self == ActionController::API
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I could have also changed it to be self.name == "ActionController::API" to avoid breaking any Rails version
Or something like:

# not sure about the logic behind constant_loaded? method
constant_loaded?("ActionController::API") && (self == ActionController::API)

But I find relying on the :action_controller_api the most elegant and correct solution loading-wise

@rafaelfranca rafaelfranca merged commit cac0dfe into rails:main Feb 8, 2022
@nvasilevski nvasilevski deleted the avoid-loading-action-controller-api-constant branch February 8, 2022 00:07
ghiculescu added a commit to ghiculescu/rabl that referenced this pull request Feb 21, 2022
that-jill referenced this pull request in powerhome/power-web-development-interview May 1, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [jbuilder](https://togithub.com/rails/jbuilder)
([changelog](https://togithub.com/rails/jbuilder/releases/tag/v2.12.0))
| `2.11.5` -> `2.12.0` |
[![age](https://developer.mend.io/api/mc/badges/age/rubygems/jbuilder/2.12.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/rubygems/jbuilder/2.12.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/rubygems/jbuilder/2.11.5/2.12.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/rubygems/jbuilder/2.11.5/2.12.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>rails/jbuilder (jbuilder)</summary>

###
[`v2.12.0`](https://togithub.com/rails/jbuilder/releases/tag/v2.12.0)

[Compare
Source](https://togithub.com/rails/jbuilder/compare/v2.11.5...v2.12.0)

#### What's Changed

- Use OpenStruct only if available by
[@&#8203;yahonda](https://togithub.com/yahonda) in
[https://github.com/rails/jbuilder/pull/562](https://togithub.com/rails/jbuilder/pull/562)
- Replace deprecated `ProxyObject` with `BasicObject` by
[@&#8203;Earlopain](https://togithub.com/Earlopain) in
[https://github.com/rails/jbuilder/pull/563](https://togithub.com/rails/jbuilder/pull/563)
- Avoid loading `ActionController::API` constant by
[@&#8203;nvasilevski](https://togithub.com/nvasilevski) in
[https://github.com/rails/jbuilder/pull/529](https://togithub.com/rails/jbuilder/pull/529)
- Fixed a bug where
[#&#8203;501](https://togithub.com/rails/jbuilder/issues/501) broke
compatibility with Enumerable by
[@&#8203;yuki24](https://togithub.com/yuki24) in
[https://github.com/rails/jbuilder/pull/531](https://togithub.com/rails/jbuilder/pull/531)
- Fix namespace issue when generating jbuilder views by
[@&#8203;hahmed](https://togithub.com/hahmed) in
[https://github.com/rails/jbuilder/pull/536](https://togithub.com/rails/jbuilder/pull/536)
- Remove reliance on ERBTracker from rails by
[@&#8203;HParker](https://togithub.com/HParker) in
[https://github.com/rails/jbuilder/pull/504](https://togithub.com/rails/jbuilder/pull/504)
- Fix require path of dependency_tracker in railtie.rb by
[@&#8203;jalyna](https://togithub.com/jalyna) in
[https://github.com/rails/jbuilder/pull/552](https://togithub.com/rails/jbuilder/pull/552)

#### New Contributors

- [@&#8203;nvasilevski](https://togithub.com/nvasilevski) made their
first contribution in
[https://github.com/rails/jbuilder/pull/529](https://togithub.com/rails/jbuilder/pull/529)
- [@&#8203;okuramasafumi](https://togithub.com/okuramasafumi) made their
first contribution in
[https://github.com/rails/jbuilder/pull/526](https://togithub.com/rails/jbuilder/pull/526)
- [@&#8203;berkos](https://togithub.com/berkos) made their first
contribution in
[https://github.com/rails/jbuilder/pull/528](https://togithub.com/rails/jbuilder/pull/528)
- [@&#8203;hahmed](https://togithub.com/hahmed) made their first
contribution in
[https://github.com/rails/jbuilder/pull/536](https://togithub.com/rails/jbuilder/pull/536)
- [@&#8203;casperisfine](https://togithub.com/casperisfine) made their
first contribution in
[https://github.com/rails/jbuilder/pull/550](https://togithub.com/rails/jbuilder/pull/550)
- [@&#8203;jalyna](https://togithub.com/jalyna) made their first
contribution in
[https://github.com/rails/jbuilder/pull/552](https://togithub.com/rails/jbuilder/pull/552)
- [@&#8203;yahonda](https://togithub.com/yahonda) made their first
contribution in
[https://github.com/rails/jbuilder/pull/562](https://togithub.com/rails/jbuilder/pull/562)
- [@&#8203;Earlopain](https://togithub.com/Earlopain) made their first
contribution in
[https://github.com/rails/jbuilder/pull/563](https://togithub.com/rails/jbuilder/pull/563)
- [@&#8203;stefannibrasil](https://togithub.com/stefannibrasil) made
their first contribution in
[https://github.com/rails/jbuilder/pull/539](https://togithub.com/rails/jbuilder/pull/539)

**Full Changelog**:
rails/jbuilder@v2.11.5...v2.12.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/powerhome/power-web-development-interview).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zMjEuMiIsInVwZGF0ZWRJblZlciI6IjM3LjMyMS4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

2 participants