Any plans to make universal library?

[kirill-konshin]

The library is potentially universal:

• xtend -> Object.assign
• popsicle -> fetch (node-fetch project for node and native/polyfill WHATWG Fetch API for browsers)
• URL & querystring parsers can be copy-pasted

I suspect that there will be a great benefit to have one OAuth Client lib for both NodeJS and Browser.

Since fetch is capable of sending forms, this will also close https://github.com/mulesoft/js-client-oauth2/issues/21

[blakeembrey]

@kirill-konshin This library is already universal, since popsicle is universal (as are all other dependencies). Use a bundler like webpack or browserify. It also wouldn't just close #21. That's a fairly simple fix and unrelated to fetch.

[kirill-konshin]

I am using webpack already.

What about size of the build? You are using Popsicle, and a couple of node specific libs that produce not optimal bundle if passed through webpack.

[blakeembrey]

@kirill-konshin Popsicle is quite small, as are the other libraries you are concerned about. Is there a specific build size you're worried about?

[kirill-konshin]

Some stats:

• Total size of webpack build is 155KB
• Size of lib itself 18KB
• Size of lib built for target:web (w/buffers etc., no popsicle) is 112KB

E.g. only 89% of the build is occupied by implementations of things that are already available in browser. NodeJS users don't care, but for browser this ~100KB make sense.

[blakeembrey]

@kirill-konshin And what about the impact on maintainers having to maintain this, having to recommend polyfills? Does the fetch polyfill you point to handle mutation. Also, a lot of the weight is from URL parsing. If you want to submit a PR, it'll be considered, but the maintainer-ship burden is usually the most expensive part and once you load in url once anyway, it'll be the same.

[blakeembrey]

Not to mention, it'll actually get larger if you just copy and paste things into every module instead of smaller. I don't personally have any desire to make the changes right now, since it'd be a major breaking change and the gains are speculative. Let me know if you do test it more completely. Also, where are buffers coming from? That is probably more additional weight that isn't required either.

[kirill-konshin]

Webpack in target:web mode bundles buffers (if used) as well as any NodeJS standard lib modules (browserified). I agree about polyfill, it's a thing that devs need to install, but since it's a standard, it's better than some custom HTTP lib...

I will look into it and make a PR.

[blakeembrey]

@kirill-konshin I would try to avoid buffers first, then things like process. I'm not even sure how you ended up with Buffer included, both this package and popsicle specify buffer: false for browsers in the package.json and Browserify respects that but I'm not as familiar with Webpack.

Also, you should be able to swap it out externally. Just use Webpack or Browserify excludes to exclude the popsicle dependency and override ClientOAuth2.request with another implementation.

[blakeembrey]

@kirill-konshin I've killed the request method that exposed the popsicle API surface. If you're passionate about it still, the dependencies could be removed for 3.0. One thing to note is the reason I switched to popsicle to start with - which is the standardised API/options re-use. There's a lot of places where we need to consistently parse as either JSON or URL encoding. Not a huge amount of manual handling, and there used to be an 0.x build without it - see https://github.com/mulesoft/js-client-oauth2/commit/2b37bbc8999780475f02d350a7a629acdb046dec (yes, this library used to be all inlined with zero dependencies, that was fun since I had extracted it from my original source - never ended up extracting my OAuth 1.0 code).

[kirill-konshin]

Fetch APIs should fit perfectly. I'll do a PR.

[blakeembrey]

@kirill-konshin I'm actually almost finished using XHR. I prefer to not use polyfills, but I'll make the PR two seconds for you to review first.

[kirill-konshin]

Cool, but XHR is unavailable in NodeJS and you'll need some implementation of it. Fetch is a standard, available in lots of browsers http://caniuse.com/#search=fetch so I really vote for it.

[blakeembrey]

@kirill-konshin I already implemented. It's ok, I've implemented it too many times now. The overhead is around 50 LOC. Fetch will make sense once IE11 and old Safaris are dead.

[blakeembrey]

Here you go - https://github.com/mulesoft/js-client-oauth2/pull/29 😄

[blakeembrey]

@kirill-konshin By the way, one necessity I didn't see on the node-fetch implementation is for rejectUnauthorized support.

[kirill-konshin]

Btw, regarding freedom of choice of network layer. Since you have injectable Promises, I think we should consider having network layer injectable too. Just look at possibilities:

• Request
• Popsicle
• Bare http/https
• Bare XHR
• Fetch
• jQuery
• Superagent
• MooTools (joke)
• etc...

Each project usually has it's own pre-configured (headers, lifecycle hooks, error handlers, loggers) layer for network, so OAuth library should be able to plug in to existing infrastructure, rather then sending requests by itself, it can have some minimalistic defaults + DI.

As an example, take a look what Backbone has: http://backbonejs.org/#Sync-ajax, by default they use jQuery, but you can override it.

[blakeembrey]

Fair enough. I went back to popsicle on the server-side and killed injected Promise in favour of globals because it's just tricky overhead - instead the request itself is the only injectable part. However, that should work fine with all the options suggested as they'd need injectable promises themselves to make it work and that's not really the way of the world.

[kirill-konshin]

From what I see in latest sources it seems that the following will work:

new ClientOAuth2({...}, function (method, url, body, headers) {
    return fetch(url, {method: method, body: body, headers: headers})
        .then(function(res){
            return Promise.all([
                res.status, 
                res.text()
            ]);
        })
        .then(function(arr){
            return {
                status: arr[0],
                body: arr[1]
            };
        });
});

[blakeembrey]

Yes, all I said was that there's no injectable promises. I said above the request can still be injected.

[kirill-konshin]

Awesome :) during our last conversation you were quite against this, so I am happy you've changed your mind. When I was writing today's comment I haven't seen latest changes, so I was surprised that it's all there already.