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

Fix missing array signature for ActiveRecordRelation #create #1946

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

bitwise-aiden
Copy link
Contributor

Motivation

If passed an array, ActiveRecord's #create method will return an array of that models type. This is missing in the current implementation.

Implementation

I've split the create methods so that the ones that are able to take arrays can now do so and return the type too.

Question for reviewers

Is there a preference on how to define constants for the single vs multi?

Tests

@bitwise-aiden bitwise-aiden requested a review from a team as a code owner July 8, 2024 05:59
@bitwise-aiden bitwise-aiden requested review from andyw8 and Morriar July 8, 2024 05:59
Copy link
Member

@paracycle paracycle left a comment

Choose a reason for hiding this comment

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

Changes look good! Can we just have comments to convey the intent behind the sig ordering?

@bitwise-aiden bitwise-aiden merged commit 7b25a9e into main Jul 16, 2024
30 checks passed
@bitwise-aiden bitwise-aiden deleted the ba-fix-arr-create branch July 16, 2024 18:40
@iMacTia
Copy link

iMacTia commented Aug 22, 2024

I read the comments around the signature order, but I don't think this is still working as expected.

After upgrading to tapioca 0.16 in our codebase, we started having issues where we call find_or_create passing T.untyped. This is because sorbet now thinks that in that scenario we get an array back, even though we're passing a single T.untyped value and therefore we're only getting back a single record.

The only solution I found was to properly type the parameter so that it wouldn't be T.untyped.
Ideally, Sorbet would distinguish between T.untyped and T::Array[T.untyped], but as the comment specifies, T.untyped matches T::Array[T.untyped].

I'm not sure this is gonna be easy to fix, as this may be due to a limitation in Sorbet, but maybe we could leverage T.anything instead? Or some other clever hack to make this work better?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants