You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@weex.apache.org by MrRaindrop <gi...@git.apache.org> on 2017/06/29 07:11:58 UTC

[GitHub] incubator-weex pull request #298: + [html5] add jsonpCallback support

Github user MrRaindrop commented on a diff in the pull request:

    https://github.com/apache/incubator-weex/pull/298#discussion_r124726568
  
    --- Diff: html5/render/browser/extend/api/stream.js ---
    @@ -12,7 +12,7 @@ let jsonpCnt = 0
     const ERROR_STATE = -1
     
     function _jsonp (config, callback, progressCallback) {
    -  const cbName = 'jsonp_' + (++jsonpCnt)
    +  const cbName = config.jsonpCallback || 'jsonp_' + (++jsonpCnt)
    --- End diff --
    
    I think it's better to use `jsonpCallbackName` instead of `jsonpCallback`, since there'are already two parameters about callback function, `callback` and `progressCallback`, in this case another `xxCallback` may mislead user to pass a jsonp callback **function** other than the callback name **string** to it according to the other `xxCallback`s are all **functions**. Using `jsonpCallbackName` should avoid this misunderstanding.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---