Breaking : Remove 'request' dependency (webhintio#5243)

This PR removes 'request' as a (direct) dependencies for the packages/* folder, this is due it being deprecated. node-fetch is used as a substitution.
luckylennoxll · Aug 4, 2022 · 3b71aaa · 3b71aaa
1 parent 233ddb0
commit 3b71aaa
Show file tree

Hide file tree

Showing 25 changed files with 313 additions and 171 deletions.
diff --git a/packages/connector-jsdom/src/connector.ts b/packages/connector-jsdom/src/connector.ts
@@ -54,6 +54,7 @@ const debug: debug.IDebugger = d(__filename);
 
 const defaultOptions = {
     ignoreHTTPSErrors: false,
+    strictSSL: false,
     waitFor: 5000
 };
 
@@ -90,10 +91,13 @@ export default class JSDOMConnector implements IConnector {
 
         const requesterOptions = {
             rejectUnauthorized: !this._options.ignoreHTTPSErrors,
-            strictSSL: !this._options.ignoreHTTPSErrors,
             ...this._options.requestOptions || {}
         };
 
+        if (this._options.strictSSL && this._options.ignoreHTTPSErrors) {
+            this._options.strictSSL = false;
+        }
+
         this.request = new Requester(requesterOptions);
         this.server = server;
         this._timeout = server.timeout;
@@ -127,9 +131,12 @@ export default class JSDOMConnector implements IConnector {
         }
 
         const r: Requester = new Requester({
-            headers: customHeaders,
-            rejectUnauthorized: !this._options.ignoreHTTPSErrors,
-            strictSSL: !this._options.ignoreHTTPSErrors
+            /*
+             * TODO: Can be strong typed with IRequesterOptions once package has
+             * been published.
+             */
+            headers: customHeaders as any,
+            rejectUnauthorized: !this._options.ignoreHTTPSErrors
         });
 
         return r.get(uri);

diff --git a/packages/connector-jsdom/src/evaluate-runner.ts b/packages/connector-jsdom/src/evaluate-runner.ts
@@ -16,7 +16,6 @@ const run = async (data: { options: any; source: string }) => {
     const { options = {}, source } = data;
     const requesterOptions = {
         rejectUnauthorized: !options.ignoreHTTPSErrors,
-        strictSSL: !options.ignoreHTTPSErrors,
         ...options.requestOptions
     };
 

diff --git a/packages/connector-puppeteer/src/connector.ts b/packages/connector-puppeteer/src/connector.ts
@@ -389,8 +389,7 @@ export default class PuppeteerConnector implements IConnector {
         const options = {
             headers,
             // we sync the ignore SSL error options with `request`. This is neeeded for local https tests
-            rejectUnauthorized: !this._options.ignoreHTTPSErrors,
-            strictSSL: !this._options.ignoreHTTPSErrors
+            rejectUnauthorized: !this._options.ignoreHTTPSErrors
         };
 
         const request = new Requester(options);

diff --git a/packages/hint-html-checker/package.json b/packages/hint-html-checker/package.json
@@ -19,9 +19,9 @@
   "description": "hint that that validates HTML pages using the Nu HTML checker",
   "devDependencies": {
     "@hint/parser-html": "^3.1.0",
+    "@hint/utils-connector-tools": "^4.0.35",
     "@hint/utils-tests-helpers": "^6.5.1",
     "@types/node": "^17.0.14",
-    "@types/request": "^2.48.8",
     "@typescript-eslint/eslint-plugin": "^4.33.0",
     "@typescript-eslint/parser": "^4.33.0",
     "ava": "^4.0.1",

diff --git a/packages/hint-html-checker/src/hint.ts b/packages/hint-html-checker/src/hint.ts
@@ -10,11 +10,11 @@
  */
 
 import uniqBy = require('lodash/uniqBy');
-import { OptionsWithUrl } from 'request';
 
 import { debug as d } from '@hint/utils-debug';
 import { HintContext, IHint } from 'hint';
 import { ProblemLocation, Severity } from '@hint/utils-types';
+import type { IRequestOptions } from '@hint/utils-connector-tools';
 
 import { HTMLEvents, HTMLParse } from '@hint/parser-html';
 
@@ -140,20 +140,20 @@ export default class HtmlCheckerHint implements IHint {
                 { severity: Severity.warning });
         };
 
-        const requestRetry = async (options: OptionsWithUrl, retries: number = 3): Promise<any> => {
+        const requestRetry = async (url: string, options: IRequestOptions, retries: number = 3): Promise<any> => {
             const requestAsync = (await import('@hint/utils-network')).requestAsync;
             const delay = (await import('@hint/utils')).delay;
 
             try {
-                return await requestAsync(options);
+                return await requestAsync(url, options);
             } catch (e) {
                 if (retries === 0) {
                     throw e;
                 }
 
                 await delay(500);
 
-                return await requestRetry(options, retries - 1);
+                return await requestRetry(url, options, retries - 1);
             }
         };
 
@@ -166,7 +166,7 @@ export default class HtmlCheckerHint implements IHint {
             return {
                 event: data,
                 failed: false,
-                promise: options.body ? requestRetry(options) : Promise.resolve({ messages: [] })
+                promise: options.body ? requestRetry(options.url, options) : Promise.resolve({ messages: [] })
             };
         };
 

diff --git a/packages/hint-html-checker/tests/tests.ts b/packages/hint-html-checker/tests/tests.ts
@@ -75,15 +75,15 @@ const configCheckerMessages = {
 const htmlCheckerMock = (response: any) => {
     return {
         '@hint/utils-network': {
-            requestAsync(scanOptions: any) {
+            requestAsync(url: string, scanOptions: any) {
                 let responseMessages;
 
                 if (response.pass) { // No errors/warnings are detected in the target html
                     return Promise.resolve(JSON.stringify(noErrorMessages));
                 }
 
                 if (response.error) { // Errors/warnings are detected in the target html
-                    const isDefaultChecker = scanOptions.url === defaultValidator;
+                    const isDefaultChecker = url === defaultValidator;
 
                     responseMessages = isDefaultChecker ? defaultCheckerMessages : configCheckerMessages;
 

diff --git a/packages/hint-html-checker/tsconfig.json b/packages/hint-html-checker/tsconfig.json
@@ -16,6 +16,7 @@
         { "path": "../hint" },
         { "path": "../parser-html" },
         { "path": "../utils" },
+        { "path": "../utils-connector-tools" },
         { "path": "../utils-debug" },
         { "path": "../utils-i18n" },
         { "path": "../utils-network" },

diff --git a/packages/hint-https-only/src/hint.ts b/packages/hint-https-only/src/hint.ts
@@ -4,7 +4,7 @@
 
 import * as URL from 'url';
 
-import { ElementFound, FetchEnd, HintContext, IHint, Response } from 'hint';
+import { ElementFound, FetchEnd, FetchError, HintContext, IHint, Response } from 'hint';
 import { isDataURI, isHTTPS } from '@hint/utils-network';
 import { debug as d } from '@hint/utils-debug';
 
@@ -105,6 +105,30 @@ export default class HttpsOnlyHint implements IHint {
             }
         };
 
+        const validateFetchError = (fetchError: FetchError) => {
+            const resource = fetchError.resource;
+
+            if (!reportedUrls.has(resource) && !isDataURI(resource) &&
+                fetchError.error && fetchError.error.code &&
+                fetchError.error.code === 'ERR_INVALID_PROTOCOL') {
+                reportedUrls.add(resource);
+
+                if (fetchError.hops && fetchError.hops.length > 0) {
+                    context.report(
+                        resource,
+                        getMessage('shouldNotBeRedirected', context.language),
+                        { severity: Severity.error }
+                    );
+                } else {
+                    context.report(
+                        resource,
+                        getMessage('shouldBeHTTPS', context.language),
+                        { severity: Severity.error }
+                    );
+                }
+            }
+        };
+
         /** Returns an array with all the URLs in the given `srcset` attribute or an empty string if none. */
         const parseSrcSet = (srcset: string | null): string[] => {
             if (!srcset) {
@@ -193,6 +217,7 @@ export default class HttpsOnlyHint implements IHint {
         };
 
         context.on('fetch::end::*', validateFetchEnd);
+        context.on('fetch::error', validateFetchError);
         context.on('element::img', validateElementSrcs);
         context.on('element::audio', validateElementSrcs);
         context.on('element::video', validateElementSrcs);

diff --git a/packages/hint-no-broken-links/package.json b/packages/hint-no-broken-links/package.json
@@ -21,7 +21,6 @@
     "@hint/utils-dom": "^2.2.0",
     "@hint/utils-tests-helpers": "^6.5.1",
     "@types/node": "^17.0.14",
-    "@types/request": "^2.48.8",
     "@typescript-eslint/eslint-plugin": "^4.33.0",
     "@typescript-eslint/parser": "^4.33.0",
     "ava": "^4.0.1",

diff --git a/packages/hint-no-broken-links/src/hint.ts b/packages/hint-no-broken-links/src/hint.ts
@@ -14,8 +14,7 @@ import { Severity } from '@hint/utils-types';
 import { isRegularProtocol } from '@hint/utils-network';
 import { HTMLElement } from '@hint/utils-dom';
 import { debug as d } from '@hint/utils-debug';
-import { Requester } from '@hint/utils-connector-tools';
-import { CoreOptions } from 'request';
+import { IRequestOptions, Requester } from '@hint/utils-connector-tools';
 
 import meta from './meta';
 import { getMessage } from './i18n.import';
@@ -33,7 +32,7 @@ export default class NoBrokenLinksHint implements IHint {
 
     public constructor(context: HintContext) {
 
-        const options: CoreOptions = { method: context.hintOptions && context.hintOptions.method ? context.hintOptions.method : 'GET' };
+        const options: IRequestOptions = { method: context.hintOptions && context.hintOptions.method ? context.hintOptions.method : 'GET' };
         const requester = new Requester(options);
         const brokenStatusCodes = [404, 410, 500, 503];
 

diff --git a/packages/hint-no-broken-links/tests/tests.ts b/packages/hint-no-broken-links/tests/tests.ts
@@ -72,15 +72,15 @@ const bodyWithMailTo = `<div>
 </div>`;
 
 const bodyWithInvalidUrl = `<div>
-<a href='http://'>About</a>
+<a href='https://'>About</a>
 </div>`;
 
 const bodyWithBrokenDnsPrefetchLinkTag = `<div>
-<link rel="dns-prefetch" href="http://localhost/404">
+<link rel="dns-prefetch" href="https://localhost/404">
 </div>`;
 
 const bodyWithBrokenPreconnectLinkTag = `<div>
-<link rel="preconnect" href="http://localhost/404">
+<link rel="preconnect" href="https://localhost/404">
 </div>`;
 
 const bodyWithInvalidDomainDnsPrefetchLinkTag = `<div>
@@ -291,7 +291,7 @@ const tests: HintTest[] = [
         name: `This test should fail as it has a loop`,
         reports: [
             {
-                message: `'http://localhost/1.mp4' could not be fetched using GET method (redirect loop detected).`,
+                message: `'https://localhost/1.mp4' could not be fetched using GET method (redirect loop detected).`,
                 severity: Severity.error
             },
             {
@@ -323,4 +323,4 @@ const tests: HintTest[] = [
     }
 ];
 
-testHint(hintPath, tests);
+testHint(hintPath, tests, {https: true});
diff --git a/packages/hint-sri/src/hint.ts b/packages/hint-sri/src/hint.ts
@@ -415,11 +415,10 @@ export default class SRIHint implements IHint {
         }
 
         try {
-            response.body.content = await requestAsync({
-                gzip: true,
+            response.body.content = await requestAsync(resource, {
+                headers: {'content-encoding': 'gzip'},
                 method: 'GET',
-                rejectUnauthorized: false,
-                url: resource
+                rejectUnauthorized: false
             });
 
             return true;

diff --git a/packages/utils-connector-tools/package.json b/packages/utils-connector-tools/package.json
@@ -16,12 +16,14 @@
     "@hint/utils-types": "^1.2.0",
     "data-urls": "^3.0.2",
     "iconv-lite": "^0.6.3",
-    "request": "^2.88.2"
+    "https": "^1.0.0",
+    "node-fetch": "2"
   },
   "description": "hint tools for connectors",
   "devDependencies": {
     "@hint/utils-create-server": "^3.4.21",
     "@types/node": "^17.0.14",
+    "@types/node-fetch": "^2.6.2",
     "@typescript-eslint/eslint-plugin": "^4.33.0",
     "@typescript-eslint/parser": "^4.33.0",
     "ava": "^4.0.1",
@@ -31,6 +33,7 @@
     "eslint-plugin-markdown": "^2.2.1",
     "npm-run-all": "^4.1.5",
     "nyc": "^15.1.0",
+    "proxyquire": "^2.1.3",
     "rimraf": "^3.0.2",
     "typescript": "^4.5.5"
   },

diff --git a/packages/utils-connector-tools/src/index.ts b/packages/utils-connector-tools/src/index.ts
@@ -2,3 +2,4 @@ export * from './normalize-headers';
 export * from './redirects';
 export * from './requester';
 export * from './resolver';
+export * from './requester-options';
diff --git a/packages/utils-connector-tools/src/requester-options.ts b/packages/utils-connector-tools/src/requester-options.ts
@@ -0,0 +1,6 @@
+import * as fetch from 'node-fetch';
+
+export interface IRequestOptions extends fetch.RequestInit {
+  rejectUnauthorized?: boolean;
+  strictSSL?: boolean;
+}