Shopify / rbi-central

MIT License
55 stars 36 forks source link

Return TimeWithZone from Time.current #262

Closed ipvalverde closed 1 month ago

ipvalverde commented 2 months ago

Type of Change

Changes

Description

Fixes https://github.com/Shopify/rbi-central/issues/261

Although the documentation does indicate that Time.current should be able to return a Time object this is unlikely to happen in a Rails application. This PR is to mitigate the need of extra code from consumers that might want to assign the return of Time.current to a TimeWithZone.

Ideally we should make the TimeWithZone to inherit from Time, since it hacks its way to be like a Time, but this was failing CI and it can be in a different PR.

paracycle commented 2 months ago

Active Support is not only used in Rails applications, so making the return type fit just a majority of Rails applications is problematic in my opinion.

Since this is dynamic and the return value depends on the presence of Time.zone, I'd rather that we create a DSL compiler for this in Tapioca which would create the right signature for the application it is working against.

paracycle commented 1 month ago

I just created a PR for the compiler, btw: https://github.com/Shopify/tapioca/pull/1962

amomchilov commented 1 month ago

Shall we close this draft then?