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

Extend support for Titanium SDK #529

Open
yuchi opened this issue Jan 14, 2015 · 15 comments
Open

Extend support for Titanium SDK #529

yuchi opened this issue Jan 14, 2015 · 15 comments

Comments

@yuchi
Copy link

yuchi commented Jan 14, 2015

At @smclab we extended the support of superagent to Titanium SDK on a project called ti-superagent.

Are the maintainers of superagent willing to accept some contribution in order to either

  1. make the required changes easier to maintain or
  2. make the package itself compatible with TitaniumSDK?

In case we chose to extend superagent’s itself, are you willing to accept to have another test environment? Those tests would run as native iOS, Android and (later this year) Windows applications.

Don’t hesitate in asking everything about either Titanium SDK, the changes we made and how we plan to have native tests running.


Some background about Titanium SDK

Titanium SDK is a platform to build cross-platform mobile apps using JavaScript and native views and components. No WebViews are harmed in a Titanium SDK app.

That means that they use just JSC on iOS and V8 on Android, and many things had to be re-implemented. Among those we can highlight Titanium.Network.HTTPClient which is an API almost, but not quite, entirely unlike XMLHttpRequest.

To make superagent work we needed to

  • apply some changes as monkey patches,
  • create a way to distribute packages written in the node de-facto standard as Titanium CommonJS modules (zipfiles with some standard files in it).

To solve the packaging issue we built titaniumifier.

@mpociot
Copy link

mpociot commented Jan 14, 2015

+1

1 similar comment
@rborn
Copy link

rborn commented Jan 14, 2015

+1

@FokkeZB
Copy link

FokkeZB commented Jan 14, 2015

+1 (excellent speech)

@defunctzombie
Copy link
Contributor

What type of changes are needed?

@yuchi
Copy link
Author

yuchi commented Jan 14, 2015

First of all something that was not explicitly stated, but it’s really important. By now Titanium SDK has no compatibility with Node.js builtins, and as a de-facto philosophy is more oriented towards reducing the learning gap for web devs rather than node devs. For that reason we’re extending and using the browser version of superagent.

We have three different kind of changes that can be made:

Changes to simplify ti-superagent maintenance

  • make the utils monkey-patchable by having them in the Request/exports obj, in particular
    • getXHR(), in Titanium you need to do Titanium.Network.createHTTPClient({ ... }),
    • parseHeader(str), to support old less xhr-complaint Titanium versions where a non-custom getResponseHeaders() returned a parsed object instead of a string;
  • split the Request#end() function body into smaller pieces so that we can (eventually) overload just specific parts of it.

Make superagent itself compatible with Titanium APIs

  • redirects(num) can be implemented in Titanium, but only as a true/false;
  • use xhr.timeout property instead of a setTimeout, so we can actually get connection timeouts but longer requests work (on mobile network this is an actual issue);
  • use xhr.onerror to catch timeouts and other kind of issues, but also check for some wrong behaviour happening on Android which fires onerror for non-2xx;
  • use xhr.onload because xhr.onreadystatechange doesn’t always fire;
  • make sure that callbacks are not fired twice, we needed to check this in ti-superagent to prevent xhr.onreadystatechange and xhr.onload collisions;
  • use xhr.send({ obj:here }) to pass Ti.Filesystem.File or Blob as attachment.

These are the most evident changes. I know that some of them are ominous, and we can (should) actually create a good test-suite and solve this from Titanium APIs standpoint. But we have many, many, many applications that are still locked to older Titanium SDK versions.

Make superagent installable as a Ti module

  • use titaniumifier to build the module zipfile that should be attached to the relative GitHub release,
  • add a "guid" property in the package.json which is required metadata,
  • add a build step for every release.

You don’t need to explicitly publish your module somewhere, gitTio (which is the module manager for Titanium) does it for you every time you push a new version and git tag.

@emilioicai
Copy link

+1

@yuchi
Copy link
Author

yuchi commented Jan 22, 2015

So, what’s your opinion on this, @defunctzombie?

@defunctzombie
Copy link
Contributor

Sorry I am really busy at work. Have not had a chance to review yet but
will try to next week.

Apologies and I do appreciate the detailed explanation!

On Thursday, January 22, 2015, Pier Paolo Ramon notifications@github.com
wrote:

So, what’s your opinion on this, @defunctzombie
https://github.com/defunctzombie?


Reply to this email directly or view it on GitHub
#529 (comment)
.

@defunctzombie
Copy link
Contributor

getXHR() is not exposed on the request object (no release yet but that code is in master)

@defunctzombie
Copy link
Contributor

parseHeader(str) and splitting .end() into smaller pieces could use PRs.

I think making you life for ti-superagent easier should be the first goal (we can do that with relative ease).

I am also onboard with most of the items in Make superagent itself compatible with Titanium APIs but some of them will need to be tweaked.

@yuchi
Copy link
Author

yuchi commented Feb 23, 2015

Great. I’ll try to transform every task in a relative PR.

@euforic
Copy link

euforic commented Mar 6, 2015

There is no need to add titanium support to superagent. All you have to do is polyfill Xhr in your TI project. Especially since the Titanium Xhr lib only deviates a little.

Ti-xhrfix.js

var Emitter = require('emitter');

// Global Context
var globalCTX = Function('return this')();

// Patch global scope

globalCTX.XMLHttpRequest = XMLHttpRequest;

// sub for browser global location property.

globalCTX.location = {};

// expose XMLHttpRequest

module.exports = XMLHttpRequest;

/**
 * XMLHttpRequest
 */

function XMLHttpRequest() {
  var self = this;
  // titanium xhr client
  this._proxy =  Ti.Network.createHTTPClient();
  this.upload = {
    onprogress: function(){}
  }
  this._proxy.onsendstream = function(e){
    self.upload.onprogress({loaded:e.progress, total:1});
  }
}

XMLHttpRequest.prototype.__proto__ = Emitter.prototype;

XMLHttpRequest.prototype.abort = function() {
  this._proxy.abort();
};

XMLHttpRequest.prototype.open = function(method, url, async) {
  this._proxy.open(method, url, async);
};

XMLHttpRequest.prototype.getResponseHeader = function(name) {
  return this._proxy.getResponseHeader(name);
};

XMLHttpRequest.prototype.send = function(data) {
  this._proxy.send(data);
};

XMLHttpRequest.prototype.setRequestHeader = function(key, val) {
  this._proxy.setRequestHeader(key, val);
};

Object.defineProperties(XMLHttpRequest.prototype, {
   'onreadystatechange' : {
      set: function (val) {
        return this._proxy.onreadystatechange = val
      }
    },
    'readyState': {
      get: function () {
        return this._proxy.readyState;
      }
    },
    'responseText': {
      get: function () {
        return this._proxy.responseText;
      }
    },
    'responseXML': {
      get: function () {
        return this._proxy.responseXML;
      }
    },
    'status': {
      get: function () {
        return this._proxy.status;
      }
    }
});

XMLHttpRequest.prototype.getAllResponseHeaders = function() {
  return '';
};

Then you can just wrap it all

ti-superagent.js

require('./TI-xhrfix');
module.exports = require('superagent');

@yuchi
Copy link
Author

yuchi commented Mar 6, 2015

This is in fact a great solution, but you’re not dealing with timeouts and Android ≠2xx issue.
Could be nonetheless the right direction.

@yuchi
Copy link
Author

yuchi commented Mar 6, 2015

BTW, where does that code comes from? Could add a small CC license to it or refer to the original author? I’d like to use it in the future.

@euforic
Copy link

euforic commented Mar 6, 2015

I wrote it over a year ago. Had it on github but i no longer use titanium so it got dropped on accident. It was MIT though so go nuts , :-) I have a better version with timeouts and multipart I just have to dig it up. I will put it back up on github when I find it.

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

7 participants