Using XMLHttpRequests results in broken 'fs' dependency

[icosamuel]

I'm trying to use your api client in the browser. So far i've been unable to build my SPA because of a dependency thats not available in the browser (fs).

$ npm install --save fs
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@1.1.3 (node_modules/fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@1.1.3: wanted {"os":"darwin","arch":"any"} (current: {"os":"linux","arch":"x64"})

+ fs@0.0.1-security
updated 1 package in 17.193s

This dependency was not found:

* fs in ./node_modules/gitlab/dist/latest/services/Projects.js, ./node_modules/request/lib/har.js

To install it, you can run: npm install --save fs

It seems ratter normal that fs cannot be used in the browser [1] [2].

What can I do to work around this?

If no workarounds are possible, Is there anyone else encountering the same issue and should we try to make a browser-friendly version?

[jdalrymple]

Wait the standard node fs library isn't available but the other ones are ? :P something about that is off

[jdalrymple]

Hmm. Lemme look

[jdalrymple]

It's only used for the upload function, to read in files. Could switch it with another library but I'm not sure that would solve your problem

[icosamuel]

There is also a reference in the request module that acts as a dependency to this project. I'm not sure either how to go about solving this

[jdalrymple]

Me neither :s I'll do some research tonight

[jdalrymple]

So there are a couple options that ive seen so far:

1. compile a browser friendly version using broswerify or something of that ilk
2. Finding a browser friendly fs library that works both natively with node and with the browser.

I've spent a few hours on 2, since i think it would be the cleanest option but havent found anything as of yet. I've also looked into one, but am having some trouble combining babel and browserify. I think i should be able to figure it out with a couple more hours of tinkering however, but if you can think of any other solutions im all ears!

[icosamuel]

So I've spent a couple of hours migrating the http-request lib to Axios. In the process, I eliminated all dependencies to fs. So I guess this would count as option 1, but not easily mergable.

I've forked the project to meet my needs, but I'd love for some ideas on how to contribute to your project. Here it is: icosamuel#1

[jdalrymple]

@Mr-Wallet any suggestions?

[Mr-Wallet]

The best option would be to abstract the request layer so that clients could inject their own dependency. Basically BaseService would take in a requester that must implement certain methods, and the interface would be basically the same as the request library, and if the client wants to use something other than request (e.g. xhr, or something that doesn't rely on fs), it would be the client's responsibility to write the adapter. This is what I probably ought to have done when adding XHR support to this library but it would have taken longer.

If the way that the request module is loaded is hacky enough, it may be able to stay a direct dependency as a "default" when no requester is provided, but won't actually blow up from a missing native Node module at runtime because it won't actually get loaded (as long as another requester was provided).

[icosamuel]

Not sure how that would be done, but there is also one call (upload i think) in services/Projects.js that requires the use of fs.

If I may suggest, Axios is getting an incredible amount of love on github. It works for node and the browser and has promises included. Do you think it is viable option to consider migrating to?

[Mr-Wallet]

That may be a superior default; just remember that today's Axios is tomorrow's dead project. I would recommend building the abstraction layer now.

My team uses node-gitlab for a desktop application that can't update at the drop of a hat and which hundreds of thousands of people rely on. Axios is moving a little fast (still has a mediocre stars-to-issues ratio,* breaking change every couple months) and while it's been around long enough that I'm sure most of the worst kinks have been worked out, I personally would feel a lot safer with a mechanism to drop Axios if anything goes wrong in my use case, without having to wrestle a solution into a personal fork the way we had to do with this project's predecessor or the way @icosamuel just had to do.

* EDIT: Not that request's stars-to-issues is anything to brag about! HTTP modules are just hard I suppose.

[jdalrymple]

Lol I agree. I'll work on making an abstraction layer for the next major release but will keep axios as a default for simplicity

[jdalrymple]

I've started working on this. I've removed the dependency internally and am working on the abstraction of the request module. There are a couple things im unsure of how to solve:

1. Consistency of the libraries. Right now their is a helper to simplify the requests. This helper assumes the request library follows a certain API. As we've seen with xhr and axios (more so the former than the latter) there is not standard API for these libraries, which usually requires us to write a wrapper (as seem with the XMLHTTPRequester file, to maintain the same API. Is this something that would have to be done for every request module the user would want to use? Or is there another way around this?

2. How to handle streams AND promises. Right now we use request for the stream and request-promise for everything else. How would this be handled by other libraries the user decided to use?

Thanks for the input :)

[Mr-Wallet]

Well, here's my personal take:

Number 1 Is not really solvable - if you want it to work for everything, you must provide an API definition in the documentation, which the client can satisfy with a wrapper. The API could coincidentally match up with one existing library, but that's as far as you can get.

For number 2: nothing is stopping people from importing as many modules as they like in their adapter. The abstraction could handle both streams and promises, but the underlying implementation can use more than one module. I don't see any real hardship on the developer since it's a simple if check for the stream option.

[jdalrymple]

😂 #documentation sounds good! Will set up a prototype in a week or so! Woo progress

[jdalrymple]

It has been suggested that one of fetch libraries is both node and browser compatible. Thoughts?

[Mr-Wallet]

I'm not totally up to speed on that discussion but the main reason we needed XMLHttpRequest (XHR) is that we have a lot of users in professional environments with badly-formed certs and peculiar hipster proxies, neither of which they're totally in control of. The thing is, Chrome and all Electron apps can all use the same settings for certs and proxies, and using XHR in Electron causes it to handle all of that. As a developer we don't need to figure out all the ways that certs/proxies can possibly work and figure out how to handle them, we can just rely 100% on the Chromium implementation. For our customers this means: "Can you access your GitLab server's website from your machine on Chrome without Chrome giving you a certificate error? Yes? Then you're already done before you even installed GitKraken, our API integration will work." This is a huge boon for us as developers, and for our customers as people who don't know anything about how certs or proxies work and have created a configuration abomination that follows no specs and only barely, coincidentally works with their current softwares.

So my basic perspective, as an Electron developer, is "I absolutely require this module, at the bottom of all the code, to go through the browser's XMLHttpRequest". If I have no recourse for that then I have absolutely no choice but to do what I did for this project's predecessor, which is to maintain our own fork. That's my team's perspective, and I recognize that it's selfish. But for the moment this is the only proper Node module for v4 of the API and so it regrettably must try to be all things to all people. If that fetch library is friendly to XHR use then I'm all for it, but if not then the ability to inject my own "requester" would still be critical.

[jdalrymple]

So a requirement for the changes is either

• 1. A library that is xhr friendly
• 2. Full ability to substitute the request library

:)

[jdalrymple]

Ive found these two:

https://github.com/serviejs/popsicle
https://www.npmjs.com/package/xhr-request

Still looking to ensure they have all the necessary functionality

ie.
headers,
body,
qs,
formData,
resolveWithFullResponse
authorization etc

Would the Got library also work? It mentioned here its electron compatible, but isnt browser compatible.
https://github.com/sindresorhus/got#comparison

[jdalrymple]

We could also internalize this https://github.com/naugtur/xhr

[jdalrymple]

@Mr-Wallet What would be the best way for me to test that it works properly in the browser. It would be a good test to setup in travis for the future.

[icosamuel]

@jdalrymple May I suggest testing a few Gitlab endpoints through a popular front-end packager such as webpack or browserify? If I remember correctly, the problem was with libraries that are exclusive to node such as fs.

[jdalrymple]

true, @Mr-Wallet with electron are you using webpack/browserify to package this node package? Creating a test case to ensure that any library changes i make don't mess it up for browser use cases would be very helpful

[icosamuel]

If I understand correctly, you would have to run your code in a browser instead of node. Maybe some headless browser testing framework like phantomjs would help?

[jdalrymple]

Yup! I want to write a test to ensure browser compatibility is maintained. So that would include running a test script that used webpack/browserify, and a headless browser situation i think. Any guidance here would be appreciated.

At least this way when we do update the request libraries, it wont break for anyone using it within the browser.

[Mr-Wallet]

I also don't have much expertise in this area, so I'm going to ask my co-worker @implausible, who has done more research on this kind of testing, to chime in.

[implausible]

I think you're on track with webpack/browserify + a headless browser. I just recently did this for one of our websites. What I ended up doing was implementing a custom reporter (socket client) in mocha which forwarded test data over a socket back to a test driver (socket server). The socket server pretty much just spits out whatever the results of the tests are the same way that the usual mocha reporter does it and also exits with an error code if the socket receives failures. I chose this implementation strategy, because if a test were to freeze the headless browser, it would be nice to see the last output from the test before travis times out the test.

In the past I found PhantomJS to be pretty terribad with a lot of gotchas as far as browser standards go, so I usually stick to real browsers in headless mode if I can. It looks like Travis has support for headless chrome https://docs.travis-ci.com/user/chrome#headless-mode. It also looks like Jest can be configured to use a custom test reporter https://jestjs.io/docs/en/configuration.

If you don't want to use the socket system, you will still need to figure out a way to have the headless browser communicate back to the running process that the tests completed and what the results were. You should be able to output whatever is on the dom though, so you could theoretically have your customer reporter write plain text to the dom and print that out at the test exit.

Hope this helps!

[jdalrymple]

released in the next tag on npm!

[westurner]

I've tried 4.2.7 (fs, net, tls), 4.5.1 (fs, net, tls), and next (5.0.0-rc.10) (#282 "Request with GET/HEAD method cannot have body. at Ky._fetch") . Any idea which version should work in Chrome 73?

[westurner]

4.5.1 works when I add this to my webpack config as described here peaksandpies/universal-analytics#58 (comment) :

node: {
    console: true,
    fs: 'empty',
    net: 'empty',
    tls: 'empty'
  }

[jdalrymple]

@westurner sorry about that! Still working on fixing this in the new version. Just been running low on time recently :(