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

Generated JavaScript function naming collision #143

Closed
parsonsmatt opened this issue Jun 23, 2015 · 5 comments
Closed

Generated JavaScript function naming collision #143

parsonsmatt opened this issue Jun 23, 2015 · 5 comments

Comments

@parsonsmatt
Copy link
Contributor

Given an API type:

type ExampleAPI = "resources" :> Get '[Text] [String]
             :<|> "resources" :> Capture "id" Int :> Get '[Text] String

The generated jQuery looks like:

function getresources(onSuccess, onError) { /* ... */ }
function getresources(id, onSuccess, onError) { /*...*/ }

Since JavaScript doesn't have function overloading, the second function overrides the first one.

This is related to #39, and I think a solution to that issue would fix this as well. The code that I'd like to generate would look something like:

function getResource(id, onSuccess, onError) {}
function getResources(onSuccess, onError) {}

where the name generator can infer (or is told about) a singular vs collection resource and pluralize the function name appropriately. In addition to a camelCase formatter, you could pass a function to resolve name collisions. We'd also want this to be generalizable for languages with other semantics around function overloading. Java would have no problem with the above example, for instance.

I'd like to contribute here, and I have some ideas for how it might be handled. Before getting started, I'd like to know what other people's takes are on this, and what the best solution might be.

A current work-around is to give the resources different names, but this breaks RESTful resource naming in URLs: resources vs resource/:id

@alpmestan
Copy link
Contributor

Hello @parsonsmatt,

Yes, the way we generate function names is hacky at best. But we needed something, to get the ball rolling and actually publish this package. The problem being that with the huge focus of everyone on servant and servant-server, we haven't taken much time to re-engineer this bit of servant-jquery, one of the reasons, I think, being that it's not used all that much.

If you are willing to give it a shot, I can help you understand anything unclear in the code and would stay around for any question. I unfortunately don't have time to tackle this myself these days but would be more than glad to help you do it. The first thing you have to do really is "simply" to find an algorithm to generate the function name just using whatever combinators you see on the way.

The way it works now is that in many (but not all) HasJQ instances, we add something to the front of the "current name", e.g:

instance Elem JSON list => HasJQ (Delete list a) where
  type JQ (Delete list a) = AjaxReq

  jqueryFor Proxy req =
    req & funcName  %~ ("delete" <>)  -- HERE: we prepend "delete" to the current function name
        & reqMethod .~ "DELETE"

So this is all you would have to modify: all the places where we tweak funcName. The trick being that ideally we want an algorithm that will generate different names for 2 endpoints that are "different enough".

I'll let you brainstorm on this now, feel free to get in touch on IRC (#servant on freenode) or to just keep discussing this here, I'll do my best to help you land a patch for this.

@parsonsmatt
Copy link
Contributor Author

Cool! :) I'll be glad to help out. I'll study the current implementation a bit more and see other potential solutions. I'll also consider @rtrvrtg's implementation. When I've got a few ideas, I'll comment and we can discuss. Thanks!

@jkarni
Copy link
Member

jkarni commented Jul 9, 2015

@parsonsmatt This seems like a duplicate of #38 ? (Maybe that should have a more informative name, or that one should be closed in favour of this one)

@alpmestan
Copy link
Contributor

The recent work by @freezeboy largely mitigates/solves this issue. See #161 and #171. This basically lets the user tweak the way function names are generated, from the components and also provides various options to tweak the output. I believe this fixes this issue so I'm closing it, but if you're not satisfied with that feel free to reopen it and explain why it doesn't work fory ou. You can learn how this can be used by looking at the documentation in servant-js/src/Servant/JS.hs, in the master branch.

@parsonsmatt
Copy link
Contributor Author

👍 very cool! :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants