Merge pull request #3 from actions/joshmgross/code-review

Initial feedback

.github/workflows/test.yml

@@ -7,7 +7,7 @@ on:
       - '**.md'
   pull_request:
     paths-ignore:
-      - '**.md'     
+      - '**.md'
 
 jobs:
 

LICENSE

@@ -1,4 +1,4 @@
-Typed Rest Client for Node.js
+Actions Http Client for Node.js
 
 Copyright (c) GitHub, Inc.
 

__tests__/auth.test.ts

@@ -1,5 +1,4 @@
 import * as httpm from '../_out';
-import * as path from 'path';
 import * as am from '../_out/auth';
 
 describe('auth', () => {

__tests__/basics.test.ts

@@ -1,14 +1,11 @@
 import * as httpm from '../_out';
 import * as path from 'path';
-import * as am from '../_out/auth';
 import * as fs from 'fs';
-import { connect } from 'http2';
 
 let sampleFilePath: string = path.join(__dirname, 'testoutput.txt');
 
 describe('basics', () => {
     let _http: httpm.HttpClient;
-    let _httpbin: httpm.HttpClient;
 
     beforeEach(() => {
         _http = new httpm.HttpClient('http-client-tests');

__tests__/keepalive.test.ts

@@ -1,13 +1,7 @@
 import * as httpm from '../_out';
-import * as path from 'path';
-import * as am from '../_out/auth';
-import * as fs from 'fs';
-
-let sampleFilePath: string = path.join(__dirname, 'testoutput.txt');
 
 describe('basics', () => {
     let _http: httpm.HttpClient;
-    let _httpbin: httpm.HttpClient;
 
     beforeEach(() => {
         _http = new httpm.HttpClient('http-client-tests', [], { keepAlive: true });

auth.ts

@@ -11,7 +11,7 @@ export class BasicCredentialHandler implements ifm.IRequestHandler {
     }
 
     prepareRequest(options:any): void {
-        options.headers['Authorization'] = 'Basic ' + new Buffer(this.username + ':' + this.password).toString('base64');
+        options.headers['Authorization'] = 'Basic ' + Buffer.from(this.username + ':' + this.password).toString('base64');
     }
 
     // This handler cannot handle 401
@@ -57,7 +57,7 @@ export class PersonalAccessTokenCredentialHandler implements ifm.IRequestHandler
     // currently implements pre-authorization
     // TODO: support preAuth = false where it hooks on 401
     prepareRequest(options:any): void {
-        options.headers['Authorization'] = 'Basic ' + new Buffer('PAT:' + this.token).toString('base64');
+        options.headers['Authorization'] = 'Basic ' + Buffer.from('PAT:' + this.token).toString('base64');
     }
 
     // This handler cannot handle 401

index.ts

@@ -4,7 +4,6 @@ import https = require("https");
 import ifm = require('./interfaces');
 import pm = require('./proxy');
 
-let fs: any;
 let tunnel: any;
 
 export enum HttpCodes {
@@ -70,7 +69,7 @@ export function isHttps(requestUrl: string) {
 }
 
 export class HttpClient {
-    userAgent: string | null | undefined;
+    userAgent: string | undefined;
     handlers: ifm.IRequestHandler[];
     requestOptions: ifm.IRequestOptions;
 
@@ -276,8 +275,7 @@ export class HttpClient {
      */
     public requestRawWithCallback(info: ifm.IRequestInfo, data: string | NodeJS.ReadableStream, onResult: (err: any, res: ifm.IHttpClientResponse) => void): void {
         let socket;
-        
+
-        let isDataString = typeof (data) === 'string';
         if (typeof (data) === 'string') {
             info.options.headers["Content-Length"] = Buffer.byteLength(data, 'utf8');
         }

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@actions/http-client",
-  "version": "1.0.0",
+  "version": "1.0.1",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {
@@ -401,23 +401,6 @@
         "@babel/types": "^7.3.0"
       }
     },
-    "@types/events": {
-      "version": "3.0.0",
-      "resolved": "https://registry.npmjs.org/@types/events/-/events-3.0.0.tgz",
-      "integrity": "sha512-EaObqwIvayI5a8dCzhFrjKzVwKLxjoG9T6Ppd5CEo07LRKfQ8Yokw54r5+Wq7FaBQ+yXRvQAYPrHwya1/UFt9g==",
-      "dev": true
-    },
-    "@types/glob": {
-      "version": "7.1.1",
-      "resolved": "https://registry.npmjs.org/@types/glob/-/glob-7.1.1.tgz",
-      "integrity": "sha512-1Bh06cbWJUHMC97acuD6UMG29nMt0Aqz1vF3guLfG+kHHJhy3AyohZFFxYk2f7Q1SQIrNwvncxAE0N/9s70F2w==",
-      "dev": true,
-      "requires": {
-        "@types/events": "*",
-        "@types/minimatch": "*",
-        "@types/node": "*"
-      }
-    },
     "@types/istanbul-lib-coverage": {
       "version": "2.0.1",
       "resolved": "https://registry.npmjs.org/@types/istanbul-lib-coverage/-/istanbul-lib-coverage-2.0.1.tgz",
@@ -452,28 +435,12 @@
         "jest-diff": "^24.3.0"
       }
     },
-    "@types/minimatch": {
-      "version": "3.0.3",
-      "resolved": "https://registry.npmjs.org/@types/minimatch/-/minimatch-3.0.3.tgz",
-      "integrity": "sha512-tHq6qdbT9U1IRSGf14CL0pUlULksvY9OZ+5eEgl1N7t+OA3tGvNpxJCzuKQlsNgCVwbAs670L1vcVQi8j9HjnA==",
-      "dev": true
-    },
     "@types/node": {
       "version": "13.1.5",
       "resolved": "https://registry.npmjs.org/@types/node/-/node-13.1.5.tgz",
       "integrity": "sha512-wupvfmtbqRJzjCm1H2diy7wo31Gn1OzvqoxCfQuKM9eSecogzP0WTlrjdq7cf7jgSO2ZX6hxwgRPR8Wt7FA22g==",
       "dev": true
     },
-    "@types/shelljs": {
-      "version": "0.8.6",
-      "resolved": "https://registry.npmjs.org/@types/shelljs/-/shelljs-0.8.6.tgz",
-      "integrity": "sha512-svx2eQS268awlppL/P8wgDLBrsDXdKznABHJcuqXyWpSKJgE1s2clXlBvAwbO/lehTmG06NtEWJRkAk4tAgenA==",
-      "dev": true,
-      "requires": {
-        "@types/glob": "*",
-        "@types/node": "*"
-      }
-    },
     "@types/stack-utils": {
       "version": "1.0.1",
       "resolved": "https://registry.npmjs.org/@types/stack-utils/-/stack-utils-1.0.1.tgz",
@@ -2381,12 +2348,6 @@
       "integrity": "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==",
       "dev": true
     },
-    "interpret": {
-      "version": "1.2.0",
-      "resolved": "https://registry.npmjs.org/interpret/-/interpret-1.2.0.tgz",
-      "integrity": "sha512-mT34yGKMNceBQUoVn7iCDKDntA7SC6gycMAWzGx1z/CMCTV7b2AAtXlo3nRyHZ1FelRkQbQjprHSYGwzLtkVbw==",
-      "dev": true
-    },
     "invariant": {
       "version": "2.2.4",
       "resolved": "https://registry.npmjs.org/invariant/-/invariant-2.2.4.tgz",
@@ -3909,15 +3870,6 @@
         "util.promisify": "^1.0.0"
       }
     },
-    "rechoir": {
-      "version": "0.6.2",
-      "resolved": "https://registry.npmjs.org/rechoir/-/rechoir-0.6.2.tgz",
-      "integrity": "sha1-hSBLVNuoLVdC4oyWdW70OvUOM4Q=",
-      "dev": true,
-      "requires": {
-        "resolve": "^1.1.6"
-      }
-    },
     "regex-not": {
       "version": "1.0.2",
       "resolved": "https://registry.npmjs.org/regex-not/-/regex-not-1.0.2.tgz",
@@ -4177,17 +4129,6 @@
       "integrity": "sha1-2kL0l0DAtC2yypcoVxyxkMmO/qM=",
       "dev": true
     },
-    "shelljs": {
-      "version": "0.8.3",
-      "resolved": "https://registry.npmjs.org/shelljs/-/shelljs-0.8.3.tgz",
-      "integrity": "sha512-fc0BKlAWiLpwZljmOvAOTE/gXawtCoNrP5oaY7KIaQbbyHeQVg01pSEuEGvGh3HEdBU4baCD7wQBwADmM/7f7A==",
-      "dev": true,
-      "requires": {
-        "glob": "^7.0.0",
-        "interpret": "^1.0.0",
-        "rechoir": "^0.6.2"
-      }
-    },
     "shellwords": {
       "version": "0.1.1",
       "resolved": "https://registry.npmjs.org/shellwords/-/shellwords-0.1.1.tgz",

package.json

@@ -24,10 +24,8 @@
   "devDependencies": {
     "@types/jest": "^24.0.25",
     "@types/node": "^13.1.5",
-    "@types/shelljs": "^0.8.6",
     "jest": "^24.9.0",
     "nock": "^11.7.2",
-    "shelljs": "^0.8.3",
     "ts-jest": "^24.3.0",
     "typescript": "^3.7.4"
   }

proxy.ts

@@ -17,7 +17,7 @@ export function getProxyUrl(reqUrl: url.Url): url.Url {
                     bypass = true;
                     break;
             }
-        }            
+        }
     }
 
     let proxyUrl: url.Url;
@@ -34,7 +34,7 @@ export function getProxyUrl(reqUrl: url.Url): url.Url {
         proxyVar = process.env["http_proxy"] ||
                     process.env["HTTP_PROXY"];
     }
-    
+
     if (proxyVar) {
         proxyUrl = url.parse(proxyVar);
     }