Skip to content
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

Incorrect method generated for routes with same URL #48

Open
2 tasks done
Shaglock opened this issue Sep 15, 2024 · 2 comments
Open
2 tasks done

Incorrect method generated for routes with same URL #48

Shaglock opened this issue Sep 15, 2024 · 2 comments
Labels
bug: pending triage Something doesn't seem to be working, but hasn't been verified

Comments

@Shaglock
Copy link
Contributor

  • I have tried upgrading by running bundle update js_from_routes.
  • I have read the troubleshooting section before opening an issue.

Description 📖

I am using devise and omniauth gems (versions 4.9.4 and 2.1.2) in brand new rails app, I have 3 providers - google, twitter and facebook. The generated paths have incorrect methods get|post

config/routes.rb

defaults export: true do
  # All routes defined inside this block will be exported.
  devise_for :users, controllers: {
    omniauth_callbacks: "users/omniauth_callbacks"
  }
end

Output of related rails routes

  user_google_oauth2_omniauth_authorize GET|POST /users/auth/google_oauth2(.:format)                users/omniauth_callbacks#passthru
   user_google_oauth2_omniauth_callback GET|POST /users/auth/google_oauth2/callback(.:format)       users/omniauth_callbacks#google_oauth2
        user_twitter_omniauth_authorize GET|POST /users/auth/twitter(.:format)                      users/omniauth_callbacks#passthru
         user_twitter_omniauth_callback GET|POST /users/auth/twitter/callback(.:format)             users/omniauth_callbacks#twitter
       user_facebook_omniauth_authorize GET|POST /users/auth/facebook(.:format)                     users/omniauth_callbacks#passthru
        user_facebook_omniauth_callback GET|POST /users/auth/facebook/callback(.:format)            users/omniauth_callbacks#facebook

Generated result

export default {
  passthru: /* #__PURE__ */ definePathHelper("get|post", "/users/auth/google_oauth2"),
  googleOauth2: /* #__PURE__ */ definePathHelper("get|post", "/users/auth/google_oauth2/callback"),
  twitter: /* #__PURE__ */ definePathHelper("get|post", "/users/auth/twitter/callback"),
  facebook: /* #__PURE__ */ definePathHelper("get|post", "/users/auth/facebook/callback"),
};

Probably its not related to devise but to the way routes are defined and how this gem handles it.
Related code from devise gem https://github.com/heartcombo/devise/blob/72884642f5700439cc96ac560ee19a44af5a2d45/lib/devise/rails/routes.rb#L446C9-L457C1

Basically if the different HTTP methods are mapped to the same controller action this issue will appear.
For example,

match "/test", to: "home#test", via: [ :get, :post, :patch, :put, :delete ]

results in

test: /* #__PURE__ */ definePathHelper("get|post|patch|put|delete", "/test"),

So my question is, is there anything we can do with it? Maybe the gem could use the as option that was added to the route and add method as suffix?
So the result would be something like
google_oauth2_omniauth_authorize_get, google_oauth2_omniauth_authorize_post, or in case of test that would be testGet, testPost, testPatch etc...

Or maybe there is some option for different exporting for such routes that I missed?

In any case, thanks for the useful gem!

@Shaglock Shaglock added the bug: pending triage Something doesn't seem to be working, but hasn't been verified label Sep 15, 2024
@Shaglock
Copy link
Contributor Author

Shaglock commented Sep 15, 2024

I just realized there was already similar issue in github "Discussions" #37, I was searching only issues :( Sorry

Also probably related to #42

@ElMassimo
Copy link
Owner

As a temporary workaround, you could try:

# config/initializers/js_from_routes.rb

if Rails.env.development?
  # Internal: Override to prefer POST requests for dual paths.
  JsFromRoutes::Route.prepend Module.new {
    def verb
      @route.verb.split("|").last.downcase
    end
  }
end

I'd like to extend support to routes that are not following the default routing patterns, but I haven't decided how that should be exposed to the frontend.

Grouping routes by controller felt like a natural way to use them, while also allowing tree-shaking.

I'm not sure what would be a good convention when a route doesn't "belong" to any resource. The main problem with non-REST routes is that I haven't needed that in any of the apps I've worked on, and it's difficult to think about it without a concrete use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: pending triage Something doesn't seem to be working, but hasn't been verified
Projects
None yet
Development

No branches or pull requests

2 participants