Skip to content

Read a route's version, anchor and requirements from its pattern#2783

Merged
ericproulx merged 2 commits into
masterfrom
route_match_params_via_pattern
Jul 6, 2026
Merged

Read a route's version, anchor and requirements from its pattern#2783
ericproulx merged 2 commits into
masterfrom
route_match_params_via_pattern

Conversation

@ericproulx

@ericproulx ericproulx commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Rebased onto master now that #2782 is merged.

version, anchor and requirements are the attributes that shape the matcher — they're passed to Grape::Router::Pattern to build the mustermann pattern. But they were also passed to Grape::Router::Route and stored as ivars, solely to back the route.version/route.anchor/route.requirements readers. That's a redundant second copy.

Change

  • Grape::Router::Pattern now stores and exposes anchor, version, requirements (attr_reader). Its initialize signature is unchanged.
  • Grape::Router::BaseRoute delegates those three readers to @pattern, alongside the existing def_delegators :@pattern, :path, :origin. They're dropped from its attr_reader, initialize keywords and ivars.
  • Endpoint#to_routes no longer passes them to Route.new.
  • Metadata that does not shape matching — namespace, prefix, settings — stays as Route keyword arguments.

The line is principled: the pattern owns the match parameters; the route owns the metadata.

Also collapses extract_capture to a guard plus a single merge. With the format keyword gone (#2782) it just adds an optional version constraint to requirements, so the intermediate accumulator hash was noise — this is the simplification that didn't make it into #2782 before it merged.

Notes

  • grape-swagger already passes anchor/version/requirements to Pattern.new and passes none of them to Route.new, so its routes' readers become correct instead of nil.
  • GreedyRoute borrows another route's pattern, so its version/anchor/requirements now reflect that pattern rather than nil. Nothing reads them (the OPTIONS/405 suite is green), so it's invisible — noted for completeness.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

Danger Report

No issues found.

View run

@ericproulx ericproulx force-pushed the route_match_params_via_pattern branch from ec239e9 to 442088e Compare July 5, 2026 18:51

@dblock dblock left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Merge on clean.

@ericproulx ericproulx marked this pull request as draft July 5, 2026 18:58
`version`, `anchor` and `requirements` are the attributes that shape the
matcher — they are passed to `Grape::Router::Pattern` to build the
mustermann pattern. They were also passed a second time to
`Grape::Router::Route` and stored as ivars, purely to back the readers.

Expose them on `Pattern` instead (`attr_reader`) and delegate the
`BaseRoute` readers to `@pattern`, alongside the existing `path`/`origin`
delegation. `to_routes` no longer passes them to `Route.new`. Metadata
that does not shape matching — `namespace`, `prefix`, `settings` — stays
as route keyword arguments.

grape-swagger already passes these to `Pattern.new` and none of them to
`Route.new`, so its routes' readers become correct rather than `nil`.

Also collapse `extract_capture` to a guard plus a single merge. With the
`format` keyword gone (#2782) it just adds an optional `version`
constraint to `requirements`, so the intermediate accumulator was noise.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ericproulx ericproulx force-pushed the route_match_params_via_pattern branch from 442088e to 2e367f1 Compare July 5, 2026 19:00
@ericproulx ericproulx marked this pull request as ready for review July 5, 2026 20:59
@ericproulx ericproulx merged commit 7f7b9e9 into master Jul 6, 2026
69 checks passed
@ericproulx ericproulx deleted the route_match_params_via_pattern branch July 6, 2026 09:44
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.

2 participants