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

HTTP Post support for dependency declaration #50

Closed
wants to merge 3 commits into from

Conversation

dvdotsenko
Copy link

(Could not find a way to send a direct message. Communicating through Pull request and some stub code instead)

AMD spec obviously does not allow for require a dependency using an HTTP Post. Well, you can do post internally, but no way to pass the post data.

I came upon realization that I if I could get my JSON-RPC requests through curl.js somehow, along (together) with text template to render it and the JS code to drive it all it would be a happy day on my street. I, obviously, can require template text and JS, but JSON-RPC requires that I post some data (and get an answer back).

This is what I have in mind:

require(
    ['js/pagecode', 'text!template.html', ['jsonrpc!/jsonrpchanlder.ashx', dataObjToGiveToPlugin] ]
    , function(Code, Template, Data){ /* bam! */ }
)

Passing array (could be Object, does not matter much) within the dependencies array is obviously not in AMD spec, but does not break the spec adherence.

I looked at Curl.js's code and was happy (again) to find it easy to bend and add non-string as supported dependency arg. The pull request is the very minor alteration I put in to make non-strings (arrays) args work without seemingly breaking the standard functionality.

I don't actually expect you to pull, I am rather perfectly happy keeping this change in my tree only. However, wanted to pitch something like this to you for future consideration.

@dvdotsenko
Copy link
Author

Only 76da26c... is the one that contains POST-specific changes. The other commits relate to other things.

@dvdotsenko
Copy link
Author

By the way here are the two reasons I (personally, subjectively) would like to have this "Post Data" functionality for:

  • We have same-domain JSON-RPC (the proper, spec v2 kind) calls hitting server for all internal control interfaces. Piling separate callback there for data is just ugly.
  • We have a number of cross-domain data pushes in transaction capture interfaces and pushing the ugly easyXDM invocation code into a Curl.js plugin would be sweet.

@unscriptable
Copy link
Member

Hey Daniel!

This sounds like an excellent use case for a plugin like you suggest. I'm hesitant to introduce the array-as-a-module-id feature, though.

There are, however, potentially many other ways to solve this. Is the information in dataObjToGiveToPlugin static and known at design time? Load time? Or just at run time?

-- J

@dvdotsenko
Copy link
Author

re: Is the information in dataObjToGiveToPlugin static and known at design time? Load time? Or just at run time?

Runtime. It's a result of user's interactions with the control interface / choices. Usually is a key-value object with "updated" values.

The above sounds a lot like "JSONP" - which one would be able to make work with unmodified Curl.js. However, on frequent occasion the value is larger than 2k chars (updated text blurb), which, i take it is too long to be a URL arg in IE. There is no way but to go "POST" in many of our use-cases.

@dvdotsenko
Copy link
Author

re: This sounds like an excellent use case for a plugin like you suggest. I'm hesitant to introduce the array-as-a-module-id feature, though.

Somehow, once the usefulness of this is realized by the crowd, i would suspect users to be picketing outside of AMD-Implementation "building", demanding "POST" functionality of this kind. I would feel an Object (opposed to Array) would be an easier sell.

I'm going to roll with "POST"-ability patches for our own use cause I think it will make our coding lives much easier.

@unscriptable
Copy link
Member

Hey @dvdotsenko, the internals of curl.js changed significantly since you added support for POSTs and array dependencies. Actually, all of the same functions are there, but they were moved onto a core object so they could be overridden by shim modules. Take a look at the latest 0.6.1 codebase.

Regards,

-- John

This is to be used to allow "post!" plugin and variants like "jsonrpc!" that need to do HTTP POST and must have a way to SEND data along with HTTP request.
@dvdotsenko
Copy link
Author

Hi John.

Wonder if your mood changed about POST data support for require?

I indeed looked into shmming the core, but, the shim would have to copy and paste entire fetchDep function. Of 150 lines in it, only 1 changes and 4 are added. ( see dvdotsenko@6ef2686 ) Since the change is backward compatible with the "AMD standard", shimming it just seems like a mistake.

Normally I don't bug project maintainers twice with API extensions, and apologize. This time I only do it because I did not find anyone else thinking bigger than "standard" while you do - with the Promise-based extension to AMD. You have the feel for "what is right" and I would be honored to get your second opinion regarding POST data extension to AMD spec.

Our corporate control interfaces (financial institution) coding really benefited from ability to require, at once, both, the data (through JSONRPC calls - that must be HTTP POST) and the response-viewing templates+handling code. If before there was a call-back hell, now the code is sane, thanks to POST data support patch we rolled for CurlJS.

If you think noone else in the world needs HTTP POST and GET response sync, I will not bug you any more. If you think that POST support is attractive, but our implementation is ugly, state so, and I will try to improve. I do see others wanting this in an AMD loader so will try to pitch this to amd-implement as "extension" if you think this idea has legs.

@unscriptable
Copy link
Member

Hey @dvdotsenko!

I just noticed you refactored onto the latest codebase. Let me look at this again. We have a new discussion group, btw. That could be a good way to see if other people think POST support is valuable. https://groups.google.com/d/forum/cujojs

-- J

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

Successfully merging this pull request may close these issues.

None yet

2 participants