Repository: node-fetch/node-fetch
Pull request #805 (2020-05-19 18:30:45)

drop Babel (while keeping ESM)
While version 3.x of `node-fetch` is targeting Node 10.x or later, there is little or nothing to do for Babel (add `debug` option in `@babel/preset-env` to see): it only transforms optional chaining, nullish coercing, dynamic imports and ESM modules.

However, there are no mentioned language features in the current codebase, except modules, and there are way more appropriate tools for module conversion.

This PR drops Babel and 7 related dependencies in favor of `rollup`, dedicated only to module conversions and producing better output.

To do so, we also forced to fix _hackish_ named exports (named exports and properties on default export, as [introduced in this commit](https://github.com/node-fetch/node-fetch/commit/d15476bb5d60eef718592bcfd451cfd616128070#diff-b9cfc7f2cdf78a7f4b91a753d10865a2), are not the same thing) with appropriate language construction in the `index.js`

As `nyc` depends on Babel this PR also replaces `nyc` with `c8` and native V8 based code-coverage reports.

This also fixes the bug with version 3.0-beta.5 of the library that was shipped with Istanbul instrumentation included in the production code.

The produced CommonJS code is also more beautiful and performant:

```js
// ---- current 3.0.0-beta.5 as published in dist/ ----
"use strict";

Object.defineProperty(exports, "__esModule", {
  value: true
});
exports.default = fetch;

var _http = _interopRequireDefault(require("http"));

// ...

var _request = _interopRequireWildcard(require("./request"));

var _fetchError = _interopRequireDefault(require("./errors/fetch-error"));

var _abortError = _interopRequireDefault(require("./errors/abort-error"));

function _getRequireWildcardCache() { if (typeof WeakMap !== "function") return null; var cache = new WeakMap(); _getRequireWildcardCache = function () { return cache; }; return cache; }

function _interopRequireWildcard(obj) { if (obj && obj.__esModule) { return obj; } if (obj === null || typeof obj !== "object" && typeof obj !== "function") { return { default: obj }; } var cache = _getRequireWildcardCache(); if (cache && cache.has(obj)) { return cache.get(obj); } var newObj = {}; var hasPropertyDescriptor = Object.defineProperty && Object.getOwnPropertyDescriptor; for (var key in obj) { if (Object.prototype.hasOwnProperty.call(obj, key)) { var desc = hasPropertyDescriptor ? Object.getOwnPropertyDescriptor(obj, key) : null; if (desc && (desc.get || desc.set)) { Object.defineProperty(newObj, key, desc); } else { newObj[key] = obj[key]; } } } newObj.default = obj; if (cache) { cache.set(obj, newObj); } return newObj; }

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

function cov_fps9prjc6() {
  var path = "/home/xxczaki/dev/node-fetch/src/index.js";
  var hash = "a8046cbf7359c2bb2f2d15890c2c22fb8edba5f4";
  var global = new Function("return this")();
  var gcv = "__coverage__";
  var coverageData = {
//
```

code from `dist/index.cjs` after merging this PR:
```js
'use strict';

exports = module.exports = fetch;

const http = require('http');
const https = require('https');
const zlib = require('zlib');
const Stream = require('stream');
const dataURIToBuffer = require('data-uri-to-buffer');
const Blob = require('fetch-blob');
const url = require('url');

/**
 * Fetch-error.js
 *
 * FetchError interface for operational errors
 */

/**
 * Create FetchError instance
 *
 * @param   String      message      Error message for human
 * @param   String      type         Error type for machine
 * @param   Object      systemError  For Node.js system error
 * @return  FetchError
 */
class FetchError extends Error {
// ...
}
```


tinovyatkin: One thing where I expecting feedback from project maintainers is regarding extension use for imports: current Node.JS specification (included in LTS) required file extensions and I see intentions to ship 3.x as ECMAScript compatible module, so, maybe it's a good time to review this rule?
jimmywarting: unrelated... but i feel like javascript needs a breather from coming up with new syntax features so browser, node and deno can focus more on on 
1) implementing missing language syntax (like optional chaining)
2) fix stuff that can't be polyfilled (like web push in Safari, The presentation api and secure sandboxing model for NodeJS, etc) - some things can be solved by the community
3) Node should be more web spec compliant and implement some of the browser api's

I don't want to have to depend on Babel, rollup or WebPack for every new project i start to build.
tinovyatkin: @Richienb 
A `.cjs` extension is neither my invention nor a personal preference - it's documented and recommended by Node.JS documentation: https://nodejs.org/api/esm.html#esm_writing_dual_packages_while_avoiding_or_minimizing_hazards
What is your reasoning behind `.js` here?

Richienb: Since we're only compiling the `.cjs` files for distribution, it would be ok then.
tinovyatkin: `c8` uses native code coverage from V8, while `nyc` so named _code instrumentation via babel and Istanbul. It's normal than when you are starting to use more precise instruments you get better results.
LinusU: Hmmm, seems like a bug though?

https://coveralls.io/builds/30915791/source?filename=src%2Frequest.js#L243
tinovyatkin: It seems that there is an ongoing issue between `esm` and `c8`: https://github.com/bcoe/c8/issues/34
So, my proposed solution is to fix all imports and make `node-fetch` work as native ESM on Node 14 or Node LTS (done at [a5ccd20](https://github.com/node-fetch/node-fetch/pull/805/commits/a5ccd20c74b8066fba0dba7f7924a5683e703e1a)) and then run coverage only on Node 14 that runs non-transpiled code (and it results in real coverage) - [dc9e992](https://github.com/node-fetch/node-fetch/pull/805/commits/dc9e992f2051b5774351340b0bc9a2a67767e18c). Now we only need to enable GitHub Actions on this repo.

Patch:
diff --git a/.editorconfig b/.editorconfig
index 991f40f..d324ccc 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -11,3 +11,6 @@ indent_style = tab
 
 [*.md]
 trim_trailing_whitespace = false
+
+[*.yml]
+indent_style = space
diff --git a/package.json b/package.json
index 9e18398..7e61222 100644
--- a/package.json
+++ b/package.json
@@ -2,12 +2,13 @@
 	"name": "node-fetch",
 	"version": "3.0.0-beta.5",
 	"description": "A light-weight module that brings window.fetch to node.js",
-	"main": "./dist/index.js",
+	"main": "./dist/index.cjs",
 	"module": "./src/index.js",
 	"sideEffects": false,
+	"type": "module",
 	"exports": {
 		"import": "./src/index.js",
-		"require": "./dist/index.js"
+		"require": "./dist/index.cjs"
 	},
 	"files": [
 		"src",
@@ -18,9 +19,9 @@
 		"node": ">=10"
 	},
 	"scripts": {
-		"build": "babel src --out-dir dist",
-		"test": "nyc --reporter=html --reporter=text mocha --require @babel/register --throw-deprecation",
-		"coverage": "nyc report --reporter=text-lcov | coveralls",
+		"build": "rollup -c",
+		"test": "node --experimental-modules node_modules/.bin/c8 --reporter=html --reporter=text --check-coverage node --experimental-modules node_modules/.bin/mocha",
+		"coverage": "c8 report --reporter=text-lcov | coveralls",
 		"lint": "xo"
 	},
 	"repository": {
@@ -43,26 +44,20 @@
 		"url": "https://opencollective.com/node-fetch"
 	},
 	"devDependencies": {
-		"@babel/cli": "^7.8.4",
-		"@babel/core": "^7.9.0",
-		"@babel/preset-env": "^7.9.5",
-		"@babel/register": "^7.9.0",
-		"@istanbuljs/nyc-config-babel": "^3.0.0",
 		"abort-controller": "^3.0.0",
 		"abortcontroller-polyfill": "^1.4.0",
-		"babel-plugin-add-module-exports": "^1.0.2",
-		"babel-plugin-istanbul": "^6.0.0",
+		"c8": "^7.1.2",
 		"chai": "^4.2.0",
 		"chai-as-promised": "^7.1.1",
 		"chai-iterator": "^3.0.2",
 		"chai-string": "^1.5.0",
-		"coveralls": "^3.0.13",
+		"coveralls": "^3.1.0",
 		"form-data": "^3.0.0",
-		"mocha": "^7.1.1",
-		"nyc": "^15.0.1",
+		"mocha": "^7.1.2",
 		"parted": "^0.1.1",
 		"promise": "^8.1.0",
 		"resumer": "0.0.0",
+		"rollup": "^2.10.4",
 		"string-to-arraybuffer": "^1.0.2",
 		"xo": "^0.30.0"
 	},
@@ -70,6 +65,9 @@
 		"data-uri-to-buffer": "^3.0.0",
 		"fetch-blob": "^1.0.5"
 	},
+	"esm": {
+		"sourceMap": true
+	},
 	"xo": {
 		"envs": [
 			"node",
@@ -81,7 +79,10 @@
 			"no-mixed-operators": 0,
 			"no-negated-condition": 0,
 			"unicorn/prevent-abbreviations": 0,
-			"@typescript-eslint/prefer-readonly-parameter-types": 0
+			"@typescript-eslint/prefer-readonly-parameter-types": 0,
+			"import/extensions": 0,
+			"import/no-useless-path-segments": 0,
+			"unicorn/import-index": 0
 		},
 		"ignores": [
 			"dist",
@@ -109,24 +110,5 @@
 			}
 		]
 	},
-	"babel": {
-		"presets": [
-			[
-				"@babel/preset-env",
-				{
-					"targets": {
-						"node": true
-					}
-				}
-			]
-		],
-		"plugins": [
-			"add-module-exports",
-			"istanbul"
-		]
-	},
-	"nyc": {
-		"extends": "@istanbuljs/nyc-config-babel"
-	},
 	"runkitExampleFilename": "example.js"
 }
diff --git a/rollup.config.js b/rollup.config.js
new file mode 100644
index 0000000..a2a5861
--- /dev/null
+++ b/rollup.config.js
@@ -0,0 +1,18 @@
+import {builtinModules} from 'module';
+import {dependencies} from './package.json';
+
+export default {
+	input: 'src/index.js',
+	output: {
+		file: 'dist/index.cjs',
+		format: 'cjs',
+		esModule: false,
+		interop: false,
+		sourcemap: true,
+		preferConst: true,
+		exports: 'named',
+		// https://github.com/rollup/rollup/issues/1961#issuecomment-534977678
+		intro: 'exports = module.exports = fetch;'
+	},
+	external: [...builtinModules, ...Object.keys(dependencies)]
+};
diff --git a/src/body.js b/src/body.js
index 9d19c89..0dd47c1 100644
--- a/src/body.js
+++ b/src/body.js
@@ -8,8 +8,8 @@
 import Stream, {PassThrough} from 'stream';
 
 import Blob from 'fetch-blob';
-import FetchError from './errors/fetch-error';
-import {isBlob, isURLSearchParams, isArrayBuffer, isAbortError} from './utils/is';
+import FetchError from './errors/fetch-error.js';
+import {isBlob, isURLSearchParams, isArrayBuffer, isAbortError} from './utils/is.js';
 
 const INTERNALS = Symbol('Body internals');
 
diff --git a/src/index.js b/src/index.js
index b364099..0f58361 100644
--- a/src/index.js
+++ b/src/index.js
@@ -12,12 +12,18 @@ import zlib from 'zlib';
 import Stream, {PassThrough, pipeline as pump} from 'stream';
 import dataURIToBuffer from 'data-uri-to-buffer';
 
-import Body, {writeToStream, getTotalBytes} from './body';
-import Response from './response';
-import Headers, {createHeadersLenient} from './headers';
-import Request, {getNodeRequestOptions} from './request';
-import FetchError from './errors/fetch-error';
-import AbortError from './errors/abort-error';
+import Body, {writeToStream, getTotalBytes} from './body.js';
+import Response from './response.js';
+import Headers, {createHeadersLenient} from './headers.js';
+import Request, {getNodeRequestOptions} from './request.js';
+import FetchError from './errors/fetch-error.js';
+import AbortError from './errors/abort-error.js';
+
+export {default as Headers} from './headers.js';
+export {default as Request} from './request.js';
+export {default as Response} from './response.js';
+export {default as FetchError} from './errors/fetch-error.js';
+export {default as AbortError} from './errors/abort-error.js';
 
 /**
  * Fetch function
@@ -305,8 +311,3 @@ fetch.isRedirect = code => [301, 302, 303, 307, 308].includes(code);
 
 // Expose Promise
 fetch.Promise = global.Promise;
-fetch.Headers = Headers;
-fetch.Request = Request;
-fetch.Response = Response;
-fetch.FetchError = FetchError;
-fetch.AbortError = AbortError;
diff --git a/src/request.js b/src/request.js
index 6e879f5..6ccdec7 100644
--- a/src/request.js
+++ b/src/request.js
@@ -9,10 +9,10 @@
 
 import {format as formatUrl} from 'url';
 import Stream from 'stream';
-import Headers, {exportNodeCompatibleHeaders} from './headers';
-import Body, {clone, extractContentType, getTotalBytes} from './body';
-import {isAbortSignal} from './utils/is';
-import {getSearch} from './utils/get-search';
+import Headers, {exportNodeCompatibleHeaders} from './headers.js';
+import Body, {clone, extractContentType, getTotalBytes} from './body.js';
+import {isAbortSignal} from './utils/is.js';
+import {getSearch} from './utils/get-search.js';
 
 const INTERNALS = Symbol('Request internals');
 
diff --git a/src/response.js b/src/response.js
index a7ec567..1fb6aba 100644
--- a/src/response.js
+++ b/src/response.js
@@ -4,8 +4,8 @@
  * Response class provides content decoding
  */
 
-import Headers from './headers';
-import Body, {clone, extractContentType} from './body';
+import Headers from './headers.js';
+import Body, {clone, extractContentType} from './body.js';
 
 const INTERNALS = Symbol('Response internals');
 


Test patch:
diff --git a/test/external-encoding.js b/test/external-encoding.js
index b7a3137..3bfd1d8 100644
--- a/test/external-encoding.js
+++ b/test/external-encoding.js
@@ -1,4 +1,4 @@
-import fetch from '../src';
+import fetch from '../src/index.js';
 import chai from 'chai';
 
 const {expect} = chai;
diff --git a/test/headers.js b/test/headers.js
index ef7b705..ef6973d 100644
--- a/test/headers.js
+++ b/test/headers.js
@@ -1,4 +1,4 @@
-import {Headers} from '../src';
+import {Headers} from '../src/index.js';
 import chai from 'chai';
 
 const {expect} = chai;
diff --git a/test/main.js b/test/main.js
index 81e1273..1902d18 100644
--- a/test/main.js
+++ b/test/main.js
@@ -2,10 +2,9 @@
 import zlib from 'zlib';
 import crypto from 'crypto';
 import {spawn} from 'child_process';
-import * as http from 'http';
-import * as fs from 'fs';
-import * as path from 'path';
-import * as stream from 'stream';
+import http from 'http';
+import fs from 'fs';
+import stream from 'stream';
 import {lookup} from 'dns';
 import vm from 'vm';
 import chai from 'chai';
@@ -17,29 +16,32 @@ import resumer from 'resumer';
 import FormData from 'form-data';
 import stringToArrayBuffer from 'string-to-arraybuffer';
 
-import {AbortController} from 'abortcontroller-polyfill/dist/abortcontroller';
+import polyfill from 'abortcontroller-polyfill/dist/abortcontroller.js';
 import AbortController2 from 'abort-controller';
 
+const {AbortController} = polyfill;
+
 // Test subjects
 import Blob from 'fetch-blob';
+
 import fetch, {
 	FetchError,
 	Headers,
 	Request,
 	Response
-} from '../src';
-import FetchErrorOrig from '../src/errors/fetch-error';
-import HeadersOrig, {createHeadersLenient} from '../src/headers';
-import RequestOrig from '../src/request';
-import ResponseOrig from '../src/response';
-import Body, {getTotalBytes, extractContentType} from '../src/body';
-import TestServer from './utils/server';
+} from '../src/index.js';
+import FetchErrorOrig from '../src/errors/fetch-error.js';
+import HeadersOrig, {createHeadersLenient} from '../src/headers.js';
+import RequestOrig from '../src/request.js';
+import ResponseOrig from '../src/response.js';
+import Body, {getTotalBytes, extractContentType} from '../src/body.js';
+import TestServer from './utils/server.js';
 
 const {
 	Uint8Array: VMUint8Array
 } = vm.runInNewContext('this');
 
-import chaiTimeout from './utils/chai-timeout';
+import chaiTimeout from './utils/chai-timeout.js';
 
 chai.use(chaiPromised);
 chai.use(chaiIterator);
@@ -1378,7 +1380,7 @@ describe('node-fetch', () => {
 
 	itIf(process.platform !== 'win32')('should allow POST request with form-data using stream as body', () => {
 		const form = new FormData();
-		form.append('my_field', fs.createReadStream(path.join(__dirname, './utils/dummy.txt')));
+		form.append('my_field', fs.createReadStream('test/utils/dummy.txt'));
 
 		const url = `${base}multipart`;
 		const options = {
diff --git a/test/request.js b/test/request.js
index 5818d64..9b38d5d 100644
--- a/test/request.js
+++ b/test/request.js
@@ -1,14 +1,16 @@
-import * as stream from 'stream';
-import * as http from 'http';
-import {Request} from '../src';
-import TestServer from './utils/server';
-import {AbortController} from 'abortcontroller-polyfill/dist/abortcontroller';
+import stream from 'stream';
+import http from 'http';
+import polyfill from 'abortcontroller-polyfill/dist/abortcontroller.js';
 import chai from 'chai';
 import FormData from 'form-data';
 import Blob from 'fetch-blob';
 import resumer from 'resumer';
 import stringToArrayBuffer from 'string-to-arraybuffer';
 
+import TestServer from './utils/server.js';
+import {Request} from '../src/index.js';
+
+const {AbortController} = polyfill;
 const {expect} = chai;
 
 const local = new TestServer();
diff --git a/test/response.js b/test/response.js
index 35a8090..1e523b8 100644
--- a/test/response.js
+++ b/test/response.js
@@ -1,10 +1,10 @@
 import * as stream from 'stream';
-import {Response} from '../src';
-import TestServer from './utils/server';
 import chai from 'chai';
 import resumer from 'resumer';
 import stringToArrayBuffer from 'string-to-arraybuffer';
 import Blob from 'fetch-blob';
+import {Response} from '../src/index.js';
+import TestServer from './utils/server.js';
 
 const {expect} = chai;
 
diff --git a/test/utils/server.js b/test/utils/server.js
index 0616d35..d03d54b 100644
--- a/test/utils/server.js
+++ b/test/utils/server.js
@@ -1,6 +1,7 @@
-import * as http from 'http';
-import * as zlib from 'zlib';
-import {multipart as Multipart} from 'parted';
+import http from 'http';
+import zlib from 'zlib';
+import parted from 'parted';
+const {multipart: Multipart} = parted;
 
 export default class TestServer {
 	constructor() {
@@ -379,9 +380,3 @@ export default class TestServer {
 	}
 }
 
-if (require.main === module) {
-	const server = new TestServer();
-	server.start(() => {
-		console.log(`Server started listening at port ${server.port}`);
-	});
-}