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

support for AjaxObservable on node. #2099

Closed
benmonro opened this issue Nov 2, 2016 · 32 comments
Closed

support for AjaxObservable on node. #2099

benmonro opened this issue Nov 2, 2016 · 32 comments

Comments

@benmonro
Copy link

benmonro commented Nov 2, 2016

RxJS version:
5

Code to reproduce:

AjaxObservable.create({
                    url,
                    method: 'GET',
                    responseType: 'json',
                    ...options
                });

Expected behavior:
that it send the request, even on the server side (i.e. nodejs)
Actual behavior:
Error saying 'your browser isn't supported'

Additional information:

@kwonoj
Copy link
Member

kwonoj commented Nov 2, 2016

Would you mind to share some snippets to show how did you fill in XMLHttpRequest in your node.js side code?

@benmonro
Copy link
Author

benmonro commented Nov 2, 2016

I didn't. I had to use isomorphic fetch, though even that has issues. I just did something like:

Observable.from(fetch(...).then(response => response.json())

Basically what I'm asking for is isomorphic AjaxObservable :)

@kwonoj
Copy link
Member

kwonoj commented Nov 2, 2016

I personally do not think that's scope of RxJS itself and consumer may need to fulfill based on isomorphic configurations, but others might have different opinion - /cc @Blesh , @jayphelps .

@benmonro
Copy link
Author

benmonro commented Nov 2, 2016

I respectfully disagree @kwonoj

If RxJS is offering AjaxObservable and it's supposed to work in Node.js, then AjaxObservable should work in node as well. Either that or it needs to be clearly defined that it's strictly a browser-only feature.

@kwonoj
Copy link
Member

kwonoj commented Nov 2, 2016

To clarify 'scope' I mentioned above, it's matter of providing XMLHttpRequest if it does not exist on certain environment. AjaxObservable will work on node as long as consumer fulfills implementation of XMLHttpRequest.

@benmonro
Copy link
Author

benmonro commented Nov 2, 2016

Is there any example/docs of this you can provide? I know there are packages in npm that support XMLHttpRequest on node, so if I can pass that in, that'd be fine as well.

@jayphelps
Copy link
Member

jayphelps commented Nov 2, 2016

@benmonro You could use something like https://www.npmjs.com/package/xmlhttprequest

// It needs to be available globally, before RxJS is loaded
global.XMLHttpRequest = require("xmlhttprequest").XMLHttpRequest;

If RxJS is offering AjaxObservable and it's supposed to work in Node.js,

It's not suppose to work in node btw. AjaxObservable is meant as DOM-specific API, AJAX is a browser-based term with XMLHttpRequest a defining characteristic of it. We will likely move it to a separate npm package at some point e.g. rxjs-dom We don't have any immediate plans to add an HTTP request helper for node.js into the core library, but never say never 😉

True isomorphic request libraries (works both on node.js and in browser) is non-trivial since there are no shared APIs. Libraries like isomorphic-fetch abstract that away, so like you discovered it's a fairly good choice for that, but doesn't support cancellation since the fetch() API doesn't.

@benmonro
Copy link
Author

benmonro commented Nov 2, 2016

well... the thing that threw me off was this in the readme.

image

Usually when a readme says "Node.js usage" it makes me think it works in node... :) anyway maybe just add something in the docs to the effect of it being intended for the browser and not node...

@benmonro
Copy link
Author

benmonro commented Nov 2, 2016

@jayphelps... the global xhr hack works. seems like you guys could add that:

if(canUseDOM === false) {
  global.XMLHttpRequest = require("xhr2");
} 

but its all good either way.

@jayphelps
Copy link
Member

Glad its working out!

@jayphelps
Copy link
Member

jayphelps commented Nov 2, 2016

Usually when a readme says "Node.js usage" it makes me think it works in node... :) anyway maybe just add something in the docs to the effect of it being intended for the browser and not node...

The observables that require a DOM are namespaced under dom.

import { ajax } from 'rxjs/observable/dom/ajax';

We're also going to remove it from the core library at some point, but since a extremely large number of people use it, it's low on our priority at the moment. Nearly everything else (minor exceptions like the rAF scheduler) work without a DOM, in node. I admit this can be confusing for some. 😞

That said, I think a proposal for a cross-platform http request utility would be awesome. It's unclear if it belongs in core rxjs though--at least until we have a better packaging story. Anyone is welcome to spearhead something by creating their own, e.g. rxjs-http and the conversation can go from there. There are some tough things though, because node's http module is much more robust than XMLHttpRequest, so presumably the ability would need to only what XMLHttpRequest supports to prevent it from not being universal.

@jayphelps jayphelps reopened this Nov 2, 2016
@benmonro
Copy link
Author

benmonro commented Nov 3, 2016

fair enough

@jeffbski
Copy link
Contributor

jeffbski commented Dec 1, 2016

I would also like to have this be isomorphic like isomorphic-fetch and axios. If I had that then I could recommend this as a better replacement. I'll try the global.XMLHttpRequest hack for now to see if that works.

@BerndWessels
Copy link

Hi guys, I ran into the same issue and would also love an isomorphic solution please.

@benmonro
Copy link
Author

That said, I think a proposal for a cross-platform http request utility would be awesome. It's unclear if it belongs in core rxjs though--at least until we have a better packaging story. Anyone is welcome to spearhead something by creating their own, e.g. rxjs-http and the conversation can go from there. There are some tough things though, because node's http module is much more robust than XMLHttpRequest, so presumably the ability would need to only what XMLHttpRequest supports to prevent it from not being universal.

@jayphelps any progress on said cross platform utility? Our little hack works, but i'm not 😍 about it.

@jayphelps
Copy link
Member

Not from me, I don't currently have a need and cycles are sparse.

@jayphelps
Copy link
Member

jayphelps commented Mar 21, 2017

Here you go 😎

import { ajax } from 'rxjs/observable/ajax';

const XHR2 = typeof XMLHttpRequest !== 'undefined'
  ? XMLHttpRequest
  : require('xhr2');

export const request = function request(options) {
  return ajax({ createXHR: () => new XHR2(), ...options });
};

// Usage

import { request } from 'something';

request({ url: '/something' })
  .subscribe(value => console.log(value));

Just publish as an npm package and call it good. :shipit:

@BerndWessels
Copy link

@jayphelps Awesome, thank you so much.

For anybody like me who comes across this and needs to do some additional ssl magic on the node server, here is how I do it:

/**
 * Isomorphic Observable.ajax request. TODO maybe make it an npm package.
 */
const request = function request(options) {
  if (process.env.WEB) {
    return Observable.ajax(options);
  } else {
    return Observable.ajax({
      createXHR: () => {
        const https = require('https');
        const XHR2 = require('xhr2');
        const xhr = new XHR2();
        const agent = new https.Agent({rejectUnauthorized: false}); // DO NOT DO rejectUnauthorized: false IN PRODUCTION !
        xhr.nodejsSet({httpsAgent: agent});
        return xhr;
      }, ...options
    });
  }
};

/**
 * This epic fetches a GraphQL query.
 */
const fetchGraphQLQueryEpic = action$ =>
  action$.ofType(ROOT_FETCH_GRAPHQL_QUERY)
    .mergeMap(action =>
      request({
        url: 'https://my-api.com/graphql',
        body: action.payload,
        method: 'POST',
        headers: {
          'Accept': 'application/json',
          'Authorization': 'Bearer ', // + accessToken,
          'Content-Type': 'application/json; charset=UTF-8'
        }
      })
        .map((payload) => normalizeGraphQLQueryResponse(payload.response.data))
        .takeUntil(action$.ofType(ROOT_FETCH_GRAPHQL_QUERY_CANCEL))
        .map(fetchGraphQLQuerySucceededCreator)
        .retry(2)
        .catch(({xhr}) => Observable.of(fetchGraphQLQueryFailedCreator(xhr)))
        .startWith(fetchGraphQLQueryPendingCreator())
    );

@Aryk
Copy link

Aryk commented Apr 17, 2017

@BerndWessels and @jayphelps , using either of your code, on the server only, the calls to Observable.ajax are returning a plain object like this:

{ _isScalar: false,
  request: 
   { async: true,
     createXHR: [Function: val],
     crossDomain: false,
     withCredentials: false,
     headers: {},
     method: 'GET',
     responseType: 'json',
     timeout: 0 } }

On the client it is returning an observable correctly.

Here is the full file for reference:

import { Observable } from 'rxjs/Observable';
import 'rxjs/add/observable/dom/ajax';

/**
 * Isomorphic Observable.ajax request.
 *
 * Code was taken from here: https://github.com/ReactiveX/rxjs/issues/2099#issuecomment-288278741
 * Basically on SSR, ajax calls will not work because it does not know how to make the XHR request
 * This, in theory, is supposed to also work with ssl.
 */
function ajax(options) {
  if (typeof XMLHttpRequest !== 'undefined') {
    return Observable.ajax(options);
  } else {
    return Observable.ajax({
      createXHR: () => {
        const https = require('https');
        const XHR2 = require('xhr2');
        const xhr = new XHR2();
        // DO NOT DO rejectUnauthorized: false IN PRODUCTION !
        const agent = new https.Agent(process.env.NODE_ENV === 'production' ? {} : {rejectUnauthorized: false});
        xhr.nodejsSet({httpsAgent: agent});
        return xhr;
      },
      ...options,
    });
  }
}

export { ajax };

Any idea why that might be happening?

@jayphelps
Copy link
Member

Seems correct--what's unexpected? Node inspect won't include non-enumerable things like methods, so it just doesn't appear to be an Observable even though it likely is--those properties are indeed on the instances of AjaxObservable.

@jayphelps
Copy link
Member

That is, have you tried subscribing to it? 😁 Not trying to be dismissive, genuinely wondering.

@Aryk
Copy link

Aryk commented Apr 17, 2017

Hey @jayphelps - I think I found the issue. On both the server and client I was calling import 'rxjs' to get all the operators. Even though this line ran on both the client and server, on the server, for some reason it did not attach the operators (but I did double-check that it was run) to the Observable prototype, but on the client all the operators are there. I'm guessing this something with how node/express is working. Maybe I'm importing rxjs too early on and Observable is getting reloaded later on and losing the operators?

Seems I have to explicitly call:

import 'rxjs/add/operator/map';

in the file when I need it (atleast on the server side). Do you know why it might not be working on the server side rendering part?

@Aryk
Copy link

Aryk commented Apr 17, 2017

I think I got to the bottom of it. Basically if I do `require 'rxjs', then I need to call this:

 * import { Observable } from 'rxjs'; 

and not this:

 * import { Observable } from 'rxjs/Observable';

If I do the second, then I need to import the needed operators when I need them.

That is my best guess so far.

@kwonoj
Copy link
Member

kwonoj commented Apr 17, 2017

@Aryk if you're importing partial operator, it is expected to import operator separately : https://github.com/ReactiveX/rxjs#es6-via-npm isn't this case?

@jayphelps
Copy link
Member

jayphelps commented Apr 17, 2017

Just for full clarity:

// imports *everything*, even though you're only
// only exposing a local binding to Observable
import { Observable } from 'rxjs';

typeof Observable.of;
// "function"

typeof Observable.prototype.map;
// "function"
// only imports Observable (and its internal dependencies)
// but no operators, unless of course you added them before or later
import { Observable } from 'rxjs/Observable';

typeof Observable.of;
// "undefined"

typeof Observable.prototype.map;
// "undefined"

@micha149
Copy link

Would it not make sense to split the dom specific code into a separate project? A little bit like react did with react-dom. So we could implement a counter part which implements the same interface and is able to run on NodeJS.

Example browser usage:

import { Observable } from 'rxjs/Observable';
import { AjaxObservable } from 'rxjs-dom/AjaxObservable/ajax';

Observable.from(ajax('https://api.example.com'));

Example node usage

import { Observable } from 'rxjs/Observable';
import { AjaxObservable } from 'rxjs-node/AjaxObservable/ajax';

Observable.from(ajax('https://api.example.com'));

@jayphelps
Copy link
Member

jayphelps commented May 17, 2017

@micha149 Indeed! 😄 it actually was separate in v4 asrx-dom but maintaining two projects is harder than it might appear because of RxJS's multiple build steps/outputs, doc generation, test suite, etc. It has been attempted several times in v5 as a monorepo and all who have tried so far gave up out of frustration and time constraints

@micha149
Copy link

😭

@mcmunder
Copy link

As suggested by @jayphelps in #2099 (comment) I published an npm package - universal-rxjs-ajax that should solve the issue. All credit goes to him of course!

@benmonro
Copy link
Author

sweet! Thanks @mcmunder. Open Source FTW

@Tomekmularczyk
Copy link

Tomekmularczyk commented Apr 6, 2018

Maybe in Rxjs v6 AjaxObservable could support Node by default so we don't have to rely on external packages?

@lock
Copy link

lock bot commented Jun 5, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants