Conversation
stathis-alexander
left a comment
There was a problem hiding this comment.
Nice, thank you for the contribution @marcus-deans !
We don't use noticed at AL, but it seems like a nice gem that I may look into using for Slack in the future.
Is there anything else to add here or good to ship?
| singleton.create_method( | ||
| 'deliver', | ||
| parameters: [ | ||
| create_opt_param('recipients', type: 'T.untyped', default: 'T.unsafe(nil)'), | ||
| create_kw_opt_param('enqueue_job', type: 'T.nilable(T::Boolean)', default: 'T.unsafe(nil)'), | ||
| create_kw_rest_param('options', type: 'T.untyped') | ||
| ], | ||
| return_type: 'void' | ||
| ) | ||
|
|
||
| singleton.create_method( | ||
| 'deliver_later', | ||
| parameters: [ | ||
| create_opt_param('recipients', type: 'T.untyped', default: 'T.unsafe(nil)'), | ||
| create_kw_opt_param('enqueue_job', type: 'T.nilable(T::Boolean)', default: 'T.unsafe(nil)'), | ||
| create_kw_rest_param('options', type: 'T.untyped') | ||
| ], | ||
| return_type: 'void' | ||
| ) |
There was a problem hiding this comment.
Not blocking, but can we use reflection to make any of these types more precise?
There was a problem hiding this comment.
upgraded to within possible options and appropriate class type - but recipients can basically be anything. very adhoc Railsy approach to things that doesn't translate well to any sort of structured programming
|
Urgh - see CI failure now. We use something like this in our Tapioca require 'active_record'
require 'active_record/connection_adapters/deduplicable'
require 'activerecord-nulldb-adapter'
# Establish a dummy database connection
ActiveRecord::Base.establish_connection(adapter: 'nulldb')Of course, want to avoid doing that here. Will do some thinking on it and see if can come up with a better solution |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
|
@marcus-deans check out how we do specs for the active record compilers we have. Copied from tapioca. Works well. |
|
@stathis-alexander got CI green but had to exclude noticed from RBI generation. specifically, it has a reliance on a Rails Engine, which I documented in the specs as well to rationalize the stubs that being said, that makes this compiler a fair amount less clean than some of the others. don't necessarily have great ideas on how to fix (as mentioned, do a bit of a hack on our local setup too). happy to just close this out if it's not a good fit overall |
Summary
Adds a DSL compiler for the Noticed gem that generates RBI files for
Noticed::EventandNoticed::Ephemeralsubclasses.Problem
The base Noticed gem RBI (from
tapioca gem noticed) provides generic return types for class methods. This causes issues:1. The
withmethod returns a generic type instead of the specific notifier instance type2. Tapioca's ActiveRecord compiler generates a
withmethod for CTEs that can shadow Noticed'swithmethodSolution
This compiler generates proper class method signatures for
with,deliver, anddeliver_lateron notifier subclasses with accurate return types:Why Instance Return Types Are Correct
The Noticed gem's class methods return instances:
with(params)callsnew(params: params)→ returns instancedeliver(...)callsnew.deliver(...)→ returns instancedeliver_lateris an alias ofdeliverMethod chaining works because
MyNotifier.with(params).deliver(user)calls:1. Class method
with→ returns instance2. Instance method
deliveron that instance → returns selfChanges
lib/tapioca/dsl/compilers/noticed.rb- New compilerspec/tapioca/dsl/compilers/noticed_spec.rb- Testsmanual/compiler_noticed.md- Documentationmanual/compilers.md- Added to compiler listGemfile- Added noticed gem (require: false) for testingsorbet/tapioca/config.yml- Excluded noticed from gem RBI generation