Published NPM Code includes ES6 Features

[Aubron]

This makes it incompatible with create-react-app and other projects.

Tested with fb and fb@next

duplicate of #147, but it was closed.

Create react app throws the following:

yarn run v1.3.2
$ react-scripts build
Creating an optimized production build...
Failed to compile.

Failed to minify the code from this file: 

 	./node_modules/fb/lib/fb.js:130 

Read more here: http://bit.ly/2tRViJ9

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Exited with code 1

runinng the file (fb.js) through JSHint shows a lot of ES6 syntax present, including the following:

634 | 'template literal syntax' is only available in ES6 (use 'esversion: 6').
681 | 'object short notation' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
686 | 'object short notation' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).

I assume it's just a babel configuration issue. I took a stab at locating it, unsuccessfully.

[dantman]

Ok, template literal and object shorthand syntax are present in the package.

This is a server side Node.js library with a minimum Node.js version of 4.x. The transpiled version of the package targets Node 4. According to http://node.green/ Node 4 supports template literals and object shorthand, so those are not transpiled because they are supported natively.

As I said, this is a Node.js based library for use on the server, this is not a client side package.

Could you explain how you are using it?

If you were writing an isomorphic web app or Electron app and just decided to bundle your code up for the server using target: 'node' or target: 'electron-...' I would understand it.

However the fact that this error directly states "Failed to minify the code from this file: " suggests that code is attempting to minify this library. There is no valid reason to minify server side code, which suggests that this library is being included in a bundle to be used in the browser, where it does not belong.

[Aubron]

I understand you, and if the intent is to keep it as a solely serversided library that's 👍

I'm using it in the context of a react application, where I send generated facebook Page tokens to the client securely as part of their login and token refresh (same as things like short lived AWS keys, etc).

What this allows me to do is have the client send images and videos (I am working on a canvas based image/video generation application), as well as get post statistics, etc, directly to/from the Facebook API, instead of having to proxy them through an intermediary API server (where this library would typically be intended for use).

But why not just use the facebook SDK?

Using the facebook SDK in create-react-app is kind of a mess. React's patterns and methodologies rely on importing modules in node style, not loading, checking for, and binding to a globally scoped javascript file. As such, for most things like this, React devs will use the node package (or repackage) of a library, and the ecosystem is geared towards this. So for someone in the situation of wanting to communicate directly with the facebook api from a react application, this is a very attractive library, when compared to the facebook SDK. Yes, this makes things weird with Node libraries that assumed they'd only be run on a node server, but most packages offer their NPM build in browser-friendly ES5. This is a common enough situation that react has a section for this issue.

Given that the only thing (that I can tell) keeping this a server-only library instead of an isomorphic library better than the Facebook SDK is babel compilation settings, I figured it would be worth bringing up.

A couple of notes:

A resolution to this issue is in the pipeline, as in 2.0 react-scripts will begin compiling node modules with the env preset: facebook/create-react-app#3815

To anyone looking for this on a < 2.0 react-scripts version, an alternative solution is to move the source files for this project into your source directory and compile them as part of your project, until you can move to >2.0.

[Aubron]

(There's a lively discussion about if shipping >ES5 in npm modules is or isn't a bad practice in facebook/create-react-app#1125, with some much smarter people than I.)

[dantman]

So you are trying to use it in the browser, but you have a reason to use it in a server-like way?

A few notes:

• Even if you get past the ES6 syntax Node 4 supports, this library still will not work in the browser. This is a Node library that uses Node's http (indirectly through another library) to make API requests. This won't work in browsers. If there was a reason to support server-like environments with browser APIs in them we'd have to refactor to use isomorphic-fetch instead.
• I understand the dislike of not being able to import Facebook's official client-side SDK. But that is the proper SDK to use for the browser. And this one for the server. Though I am contemplating supporting a fb/browser module that simply wraps the official client-side SDK letting you import it and use the extra Promise mode this library has with the official client-side SDK. See Support isomorphic web applications with a wrapper for Facebook's official client-side SDK #152
• This library does not currently support uploading videos, see how can we upload resumeable videos using this library.  #132. It requires API calls to a different graph API server. And this library's API is based around the official client-side JavaScript SDK's API, so I'm not even sure what the API to support that should be when the official SDK doesn't support it.

Edit: On the topic of ES6+ packages, I actually have a related proposal for an ecma package.json field that I opened an RFC in WebPack (webpack/webpack#6918) and Rollup for. I'd be happy to declare the ecma field I proposed in this package if build tools start using it, so any transpiling happens automatically.

As a side topic, the "everything must be ES5" target doesn't even make sense anymore. Over 80% of browser users are running browser versions that already have full native support for ES6. And for some applications that may be 100%. Pre-compiling everything to ES5 voids your ability to ship ES6 to the ES6 supporting browsers. I saved hundreds of kb in a minified bundle by importing material-ui/es (ES6) instead of material-ui (ES5) and doing the env transform myself (with an IE11 target included) + other babel optimizations. I'll save even more if I setup my build tools to generate 2 bundles, one for the 20%- of old browsers like IE11 and a separate one for the 80%+ modern/evergreen browsers that support ES6+. There are also portions of the browser ecosystem that demand ES6. You can't use ES5 classes in CustomElements without an adapter that has some quirks. And if you use <script type="module"> it's inherently ES6.

[Aubron]

if you get past the ES6 syntax Node 4 supports, this library still will not work in the browser. This is a Node library that uses Node's http (indirectly through another library) to make API requests.

Webpack (and pretty much any other bundler strapped to React) handles this for you by default, and http usage is commonplace in react libraries. In webpack, you end up with https://github.com/substack/http-browserify. I can confirm my project is using this library fine, with the exception of compilation (which I just fixed with an update to react-scripts 2.0 alpha latest 🎉 )

Though I am contemplating supporting a fb/browser module that simply wraps the official client-side SDK letting you import it and use the extra Promise mode this library has with the official client-side SDK.

That would be neat. It's still a bit noisier than I'd like to the window object, but I'll concede I'm a bit neurotic there.

This library does not currently support uploading videos,

I am aware. I added a manual multipart upload via http calls to my helper library in app. FWIW, I used a uploadVideo method in the style of the php SDK. I agree that their lack of a method for that is both perplexing and frustrating.

[dantman]

am aware. I added a manual multipart upload via http calls to my helper library in app. FWIW, I used a uploadVideo method in the style of the php SDK. I agree that their lack of a method for that is both perplexing and frustrating.

https://developers.facebook.com/bugs/

A bug report from someone who wants to upload videos from within the browser is more convincing than a SDK maintainer suggesting they add a feature there are no known users for. Then fb/browser would work for you and I'll have a base to use for implementing video uploads on the server.

[Aubron]

https://developers.facebook.com/bugs/366604830412222/

Chances of it being labeled 'we can't accept feature requests on this platform'? Probably. But I'm doing my part. 👍