-
Notifications
You must be signed in to change notification settings - Fork 69
Fix inertia routes helper #195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thank you! Just a quick question to make sure I understand the code.
lib/patches/mapper.rb
Outdated
get(route => 'inertia_rails/static#static', defaults: { component: component }) | ||
def extract_route_and_component(args) | ||
if args.is_a?(Hash) | ||
args.first |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now matter how much Ruby I write, I'm still regularly surprised by syntax 😄
lib/patches/mapper.rb
Outdated
@scope = @scope.new(module: nil) | ||
get(route, to: 'inertia_rails/static#static', defaults: { component: component }) | ||
ensure | ||
@scope = @scope.parent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain what this is doing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This overrides defaults coming from the namespace
helper. Btw, I missed the resource
helper, will update the PR to support it as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, Rails will prepend module/namespace/resource paths onto route
in get(route...
. That information is stored in @scope
. But we're already taking all that into account in extract_route_and_component
. @scope.new(module: nil)
just clears out the data that would prepend those paths, and the ensure block sets everything back for the next line in the routes file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a cleaner solution 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super clean looking!
lib/patches/mapper.rb
Outdated
if args.is_a?(Hash) | ||
args.first | ||
elsif resource_scope? | ||
[args, InertiaRails.configuration.component_path_resolver(path: [@scope[:module], @scope[:controller]].compact.join('/'), action: args)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit simplistic but I hope it's good enough:
resources :foo do
resources :bar do
inertia :baz # => pages/bar/baz.tsx
end
end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean foo
instead of pages
please?
resources :foo do
resources :bar do
inertia :baz # => foo/bar/baz.tsx
end
end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope, pages/bar/baz.tsx
mimics Rails routing: https://guides.rubyonrails.org/routing.html#nested-resources
30674cb
to
376bc8e
Compare
This PR improves
inertia
routes helper by adding support for: