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

Missing dependencies #67

Open
ryansukale opened this issue Jun 11, 2018 · 8 comments
Open

Missing dependencies #67

ryansukale opened this issue Jun 11, 2018 · 8 comments

Comments

@ryansukale
Copy link

Hey, thanks for this nice library.

It seems like when I tried to mock the post request and then make the request with a body, i got the following errors one after the other in MockXMLHttpRequest.js file.

Document is undefined
Blob is undefined
FormData is undefined

In order to make my post request work I had to do the following in my test file at the top.

import {JSDOM} from 'jsdom';
import {Blob} from 'blob-polyfill';
import FormData from 'form-data';

global.window = (new JSDOM()).window;
global.Document = window.Document;
global.Blob = Blob;
global.FormData = FormData;

My test environment is nodejs v8.9.1

Are you are missing these dependencies?

@jameslnewell
Copy link
Owner

jameslnewell commented Jun 12, 2018

Hi @ryansukale. Thanks for reporting the issue! Are you posting Blob and FormData or do you still get the errors with an ordinary post? Ordinary posts should work fine, but using FormData or one of the objects not native to nodejs will definitely cause errors. Polyfilling them is a great idea and I'd be happy to move that into the library.

Could you please provide a minimal test case to reproduce the errors. Thanks!

@ryansukale
Copy link
Author

ryansukale commented Jun 12, 2018

@jameslnewell

I am getting it with a regular post with a JSON body

const responseBody = {hello: 'world'};
    const request = {body: 'message'};
    mock.post(path, {
      status: 201,
      body: JSON.stringify(responseBody)
    });

    ajaxObservable
      .postJSON(path, request)
      .subscribe(data => {
        expect(data).to.deep.equal(responseBody);
        done();
      });
// And my postJSON is a wrapper around Rx.ajax method to post stuff, which does something like this.

return Rx.DOM.ajax({
    url,
    method: 'POST',
    responseType: 'json',
    body
  }).map(({response}) => response);

I went through the code in the MockXMLHttpRequest file.
If you see the lines - Here and the other conditions that follow, it seems to expect the Document and Blob and FormData to be present globally as long as body is present in the payload.

I am testing using mocha and sinon. Thats some more additional context if it helps.

@jameslnewell
Copy link
Owner

jameslnewell commented Jun 13, 2018

I was wondering why the tests aren't failing on nodejs... because jest sets up jsdom 🤦‍♂️

I'll work on a fix. I'm thinking I'll add additional checks for Document, Blob and FormData so that standard string bodies work, but the user is required to provide polyfills for those objects if they're not using jest AND if they have requests that rely on one/any of those objects.

What do you think?

jameslnewell pushed a commit that referenced this issue Jun 13, 2018
jameslnewell added a commit that referenced this issue Jun 14, 2018
* fix #67
@jameslnewell
Copy link
Owner

@ryansukale please try xhr-mock@2.4.1-preview.1

@jameslnewell jameslnewell reopened this Jun 14, 2018
@ryansukale
Copy link
Author

ryansukale commented Jun 15, 2018

Since its a testing framework, why not use those dependencies?
Otherwise, can you mention in the documentation that the library expects certain objects to be present. And if testing outside of jest environment, perhaps you can recommend the libraries I used?
Hunting for libraries is a pain, and would hinder adoption of this library. It would be nice to point people in some direction.

I will try with the version you specified and report back.

@ryansukale
Copy link
Author

I just tested the "xhr-mock": "2.4.1-preview.1" release and it works for my use case.
However, do consider adding the recommended libraries as part of the docs.

Thanks!

@deini
Copy link

deini commented Jul 11, 2018

Had the same issue and 2.4.1-preview.1 fixes it. 💯

@jameslnewell
Copy link
Owner

Thanks for the reminder :) I just released 2.4.1 but I won't close this issue yet as I want to document the usage with non-jsdom environments.

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

No branches or pull requests

3 participants