search

Shopify / tapioca

The swiss army knife of RBI generation
MIT License
701 stars 115 forks source link

Support array types in `typed_store` compiler #1924

Closed amomchilov closed 1 month ago

amomchilov commented 1 month ago

Motivation

Closes #1910

Implementation

Decide to refactor the #type_for method to take the whole ActiveRecord::TypedStore::Field object, instead of just its type_sym, because I would have needed to also pass in field.array. This way it can just access what it needs.

This has the nice benefit of moving the if field.null responsibility out of the decorate method.

Tests

I added tests for both array: true, null: true and array: true, null: false. I didn't assert the type of all the methods (since they're mostly the same), just the getter/setter (just like how other tests check them).

amomchilov commented 1 month ago

@ipvalverde Nice catch. I lost it somehow, but was able to revive it back from my editor's local history.

Can you have a look and review? Cheers

amomchilov commented 1 month ago

I ran these changes on core (cherry-picked in isolation, on top of the v0.14.3 version that core is currently using), and it type-checks without any changes needed:

https://github.com/Shopify/shopify/commit/36d05ee84d0084e452f644d2b07e01eb54795a8a