Skip to content

Make result arg optional in emits payload builders #35

Description

@liamfernandez

Summary

The with: payload builder in emits forces every method to accept a result argument, even when the payload doesn't depend on it. Relaxing this to make result optional brings the symbol-based form in line with how the block form already behaves and matches the convention Rails uses for before_action and friends.

Current behavior

Block-based builders are lenient (Procs ignore extra args), so this works today:

emits :user_created, on: :success do
  { source: "api", environment: Rails.env }
end

But the symbol form is strict — send(builder, result) at lib/servus/events/emitter.rb:151 requires the method to accept one positional arg:

emits :user_created, on: :success, with: :payload

private

def payload         # raises ArgumentError: wrong number of arguments (given 1, expected 0)
  { source: "api", environment: Rails.env }
end

The only way out today is to declare an unused parameter:

def payload(_result)
  { source: "api", environment: Rails.env }
end

That _result is a lie — the signature claims the method depends on the result when it doesn't. It also creates inconsistency between the symbol and block forms of the same DSL.

Proposal

Inspect the method's arity and call it with or without result accordingly. The existing branch in build_event_payload becomes:

elsif builder.is_a?(Symbol)
  method(builder).arity.zero? ? send(builder) : send(builder, result)

Behavior:

Method signature Today After
def payload(result) works works
def payload ArgumentError works
def payload(*) / def payload(**) works works (negative arity falls through to send(builder, result))

Same for blocks (already lenient) — no change there, just symmetry on paper.

Performance

Method#arity is a couple hundred nanoseconds per call. The emit path immediately invokes ActiveSupport::Notifications.instrument, which dominates by orders of magnitude (microseconds, plus all subscriber dispatch). The arity lookup is noise.

If we wanted to be pedantic, we could cache the arity on the emission hash when emits is declared:

@event_emissions[on] << {
  event_name: event_name,
  payload_builder: block || with,
  builder_arity: with && method_defined?(with) ? instance_method(with).arity : nil
}

…but that complicates registration (the method may not be defined yet when emits is called) and saves nanoseconds we don't need to save. Not worth it.

Why it's a real DX win

  1. Matches Rails idiom. before_action :authenticate doesn't force def authenticate(controller). validates ..., if: :admin? doesn't force def admin?(record). Servus is a Rails-shaped framework; it should follow the same shape.
  2. Honest signatures. Methods declare what they use. def payload is more truthful than def payload(_result) when the result is unused.
  3. Pairs cleanly with the proposed if: / unless: predicates in #34 — those will almost always be zero-arg (if: :new_connector?), so adopting the same arity convention everywhere keeps the DSL consistent.
  4. Backwards compatible. Existing def payload(result) methods keep working unchanged. The fix only relaxes a previously-rejected case.

Scope

Out of scope

  • Changing block behavior (already lenient via Proc semantics).
  • Inspecting kwargs or named parameters — keep it strictly arity-based to match how before_action works.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions