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

Don't assume nilable when #__tapioca_type is provided #2126

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

rzane
Copy link
Contributor

@rzane rzane commented Dec 13, 2024

Motivation

#__tapioca_type is meant to serve as an escape hatch to define arbitrary types for ActiveModel attributes. Currently, this escape hatch is limited, because it still assumes that all attributes are nilable.

The #__tapioca_type method should have full control over what type is emitted to the RBI file.

Implementation

This PR adds .assume_nilable? to ActiveModelTypeHelper. This method will only return false if the ActiveModel type implements #__tapioca_type.

It is the responsibility of the person who defined #__tapioca_type to wrap the type in T.nilable.

Alternatives:

  1. Allow subclasses of ActiveModel::Type::Value to implement #__tapioca_nilable?.
  2. Call .as_nilable_type in ActiveModelTypeHelper.type_for to avoid adding another public method. This is probably the simpler solution, but it would updates to the expected return values in tests.

Tests

This PR includes tests.

@rzane rzane requested a review from a team as a code owner December 13, 2024 03:27
@amomchilov amomchilov added the enhancement New feature or request label Dec 13, 2024
@amomchilov amomchilov merged commit ee847c3 into Shopify:main Dec 13, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants