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

Limit AutoRequireHook effect to be localized to loading the bundle #1529

Merged
merged 3 commits into from
Jun 9, 2023

Conversation

paracycle
Copy link
Member

@paracycle paracycle commented Jun 9, 2023

Motivation

Fix #1528

Implementation

Move AutoRequireHook to a separate file, and make it enabled only while executing a block passed to a singleton method on the module named override_require_false. The AutoRequireHook still always gets prepended to Bundler::Dependency but its behaviour in autorequire is to return very early if the module is not enabled.

Tests

Added a test that fails DSL compilation for a mock gem that is marked require: false and raises from its default require. The test passes after the fix.

@paracycle paracycle requested a review from st0012 June 9, 2023 14:53
@paracycle paracycle requested a review from a team as a code owner June 9, 2023 14:53
@paracycle paracycle requested a review from KaanOzkan June 9, 2023 14:53
@paracycle paracycle changed the title Uk fix autorequire hook Limit AutoRequireHook effect to be localized to loading the bundle Jun 9, 2023
lib/tapioca/gemfile.rb Outdated Show resolved Hide resolved
Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

Thanks 👍

end
end

sig { returns(T.untyped).checked(:never) }
Copy link
Member

Choose a reason for hiding this comment

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

TIL: checked

This commit limits the `require: false` override to only be enabled
during the execution of a block passed to the `.override_require_false`
method on `AutoRequireHook` module. This is only done while we are doing
essentially what is a `Bundler.require` call.

In all other cases, the `AutoRequireHook` module will be prepended to `Bundler::Dependency` but since it is not enabled, it will not do anything inside the `autorequire` method.
@paracycle paracycle merged commit 4eddf91 into main Jun 9, 2023
30 checks passed
@paracycle paracycle deleted the uk-fix-autorequire-hook branch June 9, 2023 16:37
@paracycle paracycle added the bug Something isn't working label Jun 12, 2023
@shopify-shipit shopify-shipit bot temporarily deployed to production June 23, 2023 18:32 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The AutoRequireHook patch should only apply to tapioca gem command
3 participants