Skip to content

Conversation

@ericproulx
Copy link
Contributor

@ericproulx ericproulx commented Dec 12, 2025

Refactor: Extract declared method into separate DSL module

Summary

This PR extracts the declared method and its related logic from Grape::DSL::InsideRoute::PostBeforeFilter into a new dedicated Grape::DSL::Declared module. This refactoring simplifies the codebase by replacing the dynamic module extension pattern (using extend) with a static include-based approach.

Changes

New Module (lib/grape/dsl/declared.rb)

  • Created Grape::DSL::Declared module: Extracts all declared method logic, including:
    • The declared public method
    • Private helper methods (declared_array, declared_hash, declared_hash_attr, handle_passed_param, optioned_param_key, optioned_declared_params)
    • The MethodNotYetAvailable exception class

Simplified InsideRoute (lib/grape/dsl/inside_route.rb)

  • Removed PostBeforeFilter module: All logic moved to Declared module
  • Removed post_filter_methods class method: No longer needed with the new pattern
  • Replaced dynamic extension with static include: Now uses include Declared instead of dynamically extending modules via extend post_extension after filter execution

Endpoint Changes (lib/grape/endpoint.rb)

  • Added @before_filter_passed instance variable: Tracks whether before filters have completed
  • Added before_filter_passed attr_reader: Allows the Declared module to check filter state
  • Set flag after before filters: @before_filter_passed = true is set after run_filters befores, :before
  • Removed dynamic module extension: No longer calls extend post_extension after running filters

Test Updates (spec/grape/dsl/inside_route_spec.rb)

  • Updated test setup: Added dummy class to test Declared module isolation

Benefits

  1. Improved code organization: declared method logic is now in its own dedicated module
  2. Simpler pattern: Replaces dynamic module extension (runtime extend) with static module inclusion (compile-time include)
  3. Better maintainability: Clearer separation of concerns makes the code easier to understand and modify
  4. Reduced complexity: Eliminates the post_filter_methods pattern in favor of a straightforward flag check

Technical Details

The declared method now checks the before_filter_passed flag directly instead of relying on dynamic module extension timing. Previously, the code used extend PostBeforeFilter dynamically after filter execution to make methods available. This has been replaced with a static include Declared that is always present, but the methods check the before_filter_passed flag to enforce the same behavior (preventing declared from being called before parameter validation). This provides the same functionality but with a simpler, more explicit, and more performant mechanism.

Testing

  • All existing tests pass
  • No breaking changes to public API
  • declared method behavior remains unchanged

Notes

This is a pure refactoring with no functional changes. The declared method's behavior and API remain exactly the same, but the internal implementation is cleaner and more maintainable. The key improvement is replacing the dynamic extend pattern with a static include pattern.

@ericproulx ericproulx force-pushed the refactor/declared_availability branch 2 times, most recently from 004ecc9 to c70e513 Compare December 12, 2025 16:50
@ericproulx ericproulx requested a review from dblock December 12, 2025 18:18
begin
self.class.run_before_each(self)
run_filters befores, :before
@before_filter_passed = true
Copy link
Member

Choose a reason for hiding this comment

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

Should we be tracking filters than ran more generically in run_filters?

Copy link
Contributor Author

@ericproulx ericproulx Dec 13, 2025

Choose a reason for hiding this comment

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

It is tracked through instrumentation but feels over engineered for 1 case

def run_filters(filters, type = :other)
  ActiveSupport::Notifications.instrument('endpoint_run_filters.grape', endpoint: self, filters: filters, type: type) do
    filters&.each { |filter| instance_eval(&filter) }
  end
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still I don't like this function and I will revisit it. There's some pluralization and dynamic define_method. I think it could be simplified but I'll do it after this merge if you don't mind

- Extract declared method logic from InsideRoute::PostBeforeFilter into new Declared module
- Simplify InsideRoute by replacing dynamic module extension with standard include
- Add before_filter_passed flag in Endpoint to track filter state
- Maintain backward compatibility by aliasing MethodNotYetAvailable exception
- Update tests to reflect new module structure
- Add CHANGELOG entry

This refactoring improves code organization and maintainability while
preserving all existing functionality and API compatibility.
@ericproulx ericproulx force-pushed the refactor/declared_availability branch from c70e513 to 9cfac04 Compare December 13, 2025 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants