Fetch: Add missing semicolons to avoid minification issues

[julmot]

This week we encountered an issue on the Bower website where we're using this fetch polyfill.

Users were complaining a syntax error in our script that was compressed using Cloudflare's "Auto minify" option. It turned out that the missing semicolons in this library are the cause of this issue. Here's the reply from CloudFlare:

Auto minify will remove unnecessary characters from your source code (like whitespace, comments, etc.) without changing its functionality.

In other words it'll not add missing ; to the end of lines. Thus, syntax errors appear.

I think that semicolons should be standard in general – even if not absolutely necessary, but I'd like to request this at least to avoid minification issues when processors are just removing the whitespace. Furthermore I'd like to request a .min.js file for this lib.

[mislav]

If a tool processes a javascript file that worked and turned it into a javascript file that doesn't work, that tool doesn't hold true to its promise of “without changing its functionality”.

It sounds like Cloudflare has still some wrinkles to iron out with is “Auto minify” option. Meanwhile, I suggest that you use a tool such as UglifyJS that can minify javascript files without breaking them. We don't provide a .min.js file because we're assuming that people are concatenating this library into their own apps via their bundling tool of choice and minifying the result as a whole.

[julmot]

Sure, we can minify files ourself and concat them into our .min.js files. But, providing files that can just be minified by removing whitespace and providing a min.js file is nothing elaborate and kina standard. What is against it?
cc @dgraham since you've created this file initially

[julmot]

Btw: By providing a .min.js file things regarding CDN's may be more easier too. The version on jsDelivr is quite old (29 May 2015 – v0.9) and was minified manually. If you'd offer the .min.js file it could be updated automatically.

[julmot]

Ping @josh @dgraham @mislav

[mislav]

I don't know anything about fetch on jsDelivr because I don't think we created that. A community member likely did, which is not very useful in the long term because it's easy to fall out of date (as demonstrated).

I see no reason why should we ship the npm package with the .min.js version, so I'm not going to be the one who adds it, but if my colleagues think that would be worthwhile, I could be convinced. For the time being, though, we're delegating the responsibility of minification to the user.

[julmot]

@mislav Thanks for your reply.

I'm not going to be the one who adds it

That wouldn't be the problem. I'm open for contributions.

[mislav]

That wouldn't be the problem. I'm open for contributions.

Oh, I wasn't referring to the cost of adding the code. In my view, this could be implemented as a simple pre-publish step for npm:

uglifyjs --compress --mangle fetch.js > fetch.min.js

And then ensuring that this file is included in the npm release would be a total of ~3 line change. Technically, this is easy. But from the maintainers perspective, it's another thing to maintain over time, and it represents an antipattern. We don't want people to use the minified version that we provide. We want them to create their own minified version.

[ErisDS]

we're assuming that people are concatenating this library into their own apps via their bundling tool of choice and minifying the result as a whole.

What about people who aren't building web apps, but websites? I have a use case where I only need the fetch polyfill + promise polyfill to make one or two ajax requests cross-browser compatible on an otherwise ridiculously simple static site.

I build other large JS apps where build tooling is the norm, and pulling a library like this into our own build chain & minifying would be both standard and trivial.

In this case, there is no build tooling in use or needed. Therefore, in order to minify the fetch polyfill I either have to manually do it, or add the concept of a "build" to a site that otherwise doesn't need it - adding complexity to CI and well, everything 😬

The promise polyfill you link to already has a .min.js version and I was wondering what it would take to convince you that this isn't an antipattern, but catering for a different use case?

[mislav]

Thanks for taking time to explain your use case to us.

I'm still not convinced that sites, either large or small, are facing a blocker in development because this npm package doesn't ship with a minified version. I don't think that this is forcing you to add a build pipeline or complicate CI setup further for your otherwise simple, static site. You can simply do a one-time bundle process in which you create a minified concatenation of all our JS dependencies and save it as a regular JS file on your site. It really isn't hard:

npm i promise-polyfill whatwg-fetch uglify-js

./node_modules/.bin/uglifyjs --compress --mangle -- \
  ./node_modules/promise-polyfill/promise.js \
  ./node_modules/whatwg-fetch/fetch.js > promise-fetch.min.js

rm -rf ./node_modules # optional cleanup

Now you have a promise-fetch.min.js that contains both polyfills and that you can check into version control, and it only took a minute, and you only have to ever do it once.

[ErisDS]

I understand what you're saying, as a repo maintainer, I fully get the point of view from which you're coming at this and that adding maintenance overhead is painful.

As a user though, my perspective is very different. The code you've posted makes assumptions. I have to have node and npm installed, with versions that will work with the libraries you're wanting me to use. I need to know to use the 3 npm libraries you've linked to (note: this issue thread started because someone used the wrong tool to minify the file).

As a frontend JS dev coming fresh to this repo, that's a LOOOOOOOT of assumed knowledge. Scary and daunting.

I won't waste any more of your time. I came to this repo, was put off by the lack of minified file, and decided that for my usecase it was just easier for now to add the ridiculous overhead of jQuery.

Instead of disappearing off without saying anything, I thought I'd take the time to let you know in case it helped someone else in future 🙂

[keithamus]

For anyone that reads through this looking for a solution for the use-case of wanting to use fetch but no having tooling, I'd recommend the use of Pollyfill.io which provides a drop-in script to update browsers to "evergreen standards" - it includes a fetch polyfill. One can simply drop the script tag in their HTML, no node, no npm, no uglify needed:

<script src="https://cdn.polyfill.io/v2/polyfill.min.js"></script>

The polyfill.io service is backed by Fastly's CDN, so it's plenty fast, it also has some smarts to it - it'll only load the polyfill for browsers which need it, rather than for every browser! (If for some reason you want to force it to always load the fetch polyfill, just add ?features=fetch|always to the URL)

If you're concerned about loading in other polyfills, and _just_ want fetch with _no other polyfills_ (other than what is required for fetch to work) then you can change the URL to the following:

<script src=https://cdn.polyfill.io/v2/polyfill.min.js?features=none,fetch"></script>

(Again, change this to ?features=none,fetch|always and it'll _always_ load fetch in every browser, even the ones that don't need it).

N.B. It's worth noting that Polyfill.io's fetch polyfill uses this repo's source.