Skip to content

Commit

Permalink
fix: feature detection to check for browser (#738)
Browse files Browse the repository at this point in the history
  • Loading branch information
JustinBeckwith committed Jun 20, 2019
1 parent 7a6c8e7 commit 83a5ba5
Show file tree
Hide file tree
Showing 14 changed files with 18 additions and 51 deletions.
6 changes: 2 additions & 4 deletions package.json
Expand Up @@ -24,8 +24,7 @@
"gcp-metadata": "^2.0.0",
"gtoken": "^3.0.0",
"jws": "^3.1.5",
"lru-cache": "^5.0.0",
"semver": "^6.0.0"
"lru-cache": "^5.0.0"
},
"devDependencies": {
"@compodoc/compodoc": "^1.1.7",
Expand All @@ -39,7 +38,6 @@
"@types/ncp": "^2.0.1",
"@types/nock": "^10.0.3",
"@types/node": "^10.5.1",
"@types/semver": "^6.0.0",
"@types/sinon": "^7.0.0",
"@types/tmp": "^0.1.0",
"assert-rejects": "^1.0.0",
Expand Down Expand Up @@ -68,7 +66,7 @@
"ncp": "^2.0.0",
"nock": "^10.0.0",
"null-loader": "^3.0.0",
"nyc": "^14.0.0",
"nyc": "^14.1.1",
"prettier": "^1.13.4",
"puppeteer": "^1.11.0",
"sinon": "^7.0.0",
Expand Down
1 change: 0 additions & 1 deletion src/auth/authclient.ts
Expand Up @@ -18,7 +18,6 @@ import {EventEmitter} from 'events';
import {GaxiosOptions, GaxiosPromise} from 'gaxios';

import {DefaultTransporter} from '../transporters';

import {Credentials} from './credentials';

export declare interface AuthClient {
Expand Down
1 change: 0 additions & 1 deletion src/auth/googleauth.ts
Expand Up @@ -23,7 +23,6 @@ import * as path from 'path';
import * as stream from 'stream';

import {createCrypto} from '../crypto/crypto';
import {isBrowser} from '../isbrowser';
import * as messages from '../messages';
import {DefaultTransporter, Transporter} from '../transporters';

Expand Down
1 change: 0 additions & 1 deletion src/auth/jwtaccess.ts
Expand Up @@ -19,7 +19,6 @@ import * as LRU from 'lru-cache';
import * as stream from 'stream';

import * as messages from '../messages';

import {JWTInput} from './credentials';
import {Headers, RequestMetadataResponse} from './oauth2client';

Expand Down
5 changes: 2 additions & 3 deletions src/auth/oauth2client.ts
Expand Up @@ -23,8 +23,7 @@ import {
import * as querystring from 'querystring';
import * as stream from 'stream';

import {createCrypto, JwkCertificate} from '../crypto/crypto';
import {isBrowser} from '../isbrowser';
import {createCrypto, JwkCertificate, hasBrowserCrypto} from '../crypto/crypto';
import * as messages from '../messages';
import {BodyResponseCallback} from '../transporters';

Expand Down Expand Up @@ -1015,7 +1014,7 @@ export class OAuth2Client extends AuthClient {

async getFederatedSignonCertsAsync(): Promise<FederatedSignonCertsResponse> {
const nowTime = new Date().getTime();
const format: CertificateFormat = isBrowser()
const format = hasBrowserCrypto()
? CertificateFormat.JWK
: CertificateFormat.PEM;
if (
Expand Down
5 changes: 1 addition & 4 deletions src/crypto/browser/crypto.ts
Expand Up @@ -19,13 +19,10 @@

import * as base64js from 'base64-js';

import {isBrowser} from '../../isbrowser';
import {CryptoSigner} from '../crypto';

// Not all browsers support `TextEncoder`. The following `require` will
// provide a fast UTF8-only replacement for those browsers that don't support
// text encoding natively.
if (isBrowser() && typeof TextEncoder === 'undefined') {
if (typeof TextEncoder === 'undefined') {

This comment has been minimized.

Copy link
@lovell

lovell Jun 25, 2019

@JustinBeckwith Was the removal of the isBrowser() check here intentional? This source file is always required when using Node and this check is now always true for Node, which means that it is also now requiring the fast-text-encoding module again.

This comment has been minimized.

Copy link
@JustinBeckwith

JustinBeckwith Jun 25, 2019

Author Contributor

Ah, looks like a bug. Could I trouble you to file an issue?

This comment has been minimized.

Copy link
@JustinBeckwith

JustinBeckwith Jun 25, 2019

Author Contributor

fix in #740

require('fast-text-encoding');
}

Expand Down
11 changes: 9 additions & 2 deletions src/crypto/crypto.ts
Expand Up @@ -14,7 +14,6 @@
* limitations under the License.
*/

import {isBrowser} from '../isbrowser';
import {BrowserCrypto} from './browser/crypto';
import {NodeCrypto} from './node/crypto';

Expand Down Expand Up @@ -56,8 +55,16 @@ export interface Crypto {
}

export function createCrypto(): Crypto {
if (isBrowser()) {
if (hasBrowserCrypto()) {
return new BrowserCrypto();
}
return new NodeCrypto();
}

export function hasBrowserCrypto() {
return (
typeof window !== 'undefined' &&
typeof window.crypto !== 'undefined' &&
typeof window.crypto.subtle !== 'undefined'
);
}
2 changes: 1 addition & 1 deletion src/crypto/node/crypto.ts
Expand Up @@ -15,7 +15,7 @@
*/

import * as crypto from 'crypto';
import {Crypto, CryptoSigner} from '../crypto';
import {Crypto} from '../crypto';

export class NodeCrypto implements Crypto {
async sha256DigestBase64(str: string): Promise<string> {
Expand Down
19 changes: 0 additions & 19 deletions src/isbrowser.ts

This file was deleted.

11 changes: 2 additions & 9 deletions src/messages.ts
Expand Up @@ -14,9 +14,6 @@
* limitations under the License.
*/

import * as semver from 'semver';
import {isBrowser} from './isbrowser';

export enum WarningTypes {
WARNING = 'Warning',
DEPRECATION = 'DeprecationWarning',
Expand All @@ -28,18 +25,14 @@ export function warn(warning: Warning) {
return;
}
warning.warned = true;
if (isBrowser()) {
console.warn(warning.message);
} else if (semver.satisfies(process.version, '>=8')) {
if (typeof process !== 'undefined' && process.emitWarning) {
// @types/node doesn't recognize the emitWarning syntax which
// accepts a config object, so `as any` it is
// https://nodejs.org/docs/latest-v8.x/api/process.html#process_process_emitwarning_warning_options
// tslint:disable-next-line no-any
process.emitWarning(warning.message, warning as any);
} else {
// This path can be removed once we drop support for Node 6.
// https://nodejs.org/docs/latest-v6.x/api/process.html#process_process_emitwarning_warning_name_ctor
process.emitWarning(warning.message, warning.type);
console.warn(warning.message);
}
}

Expand Down
4 changes: 1 addition & 3 deletions src/transporters.ts
Expand Up @@ -21,8 +21,6 @@ import {
GaxiosResponse,
request,
} from 'gaxios';

import {isBrowser} from './isbrowser';
import {validate} from './options';

// tslint:disable-next-line no-var-requires
Expand Down Expand Up @@ -60,7 +58,7 @@ export class DefaultTransporter {
*/
configure(opts: GaxiosOptions = {}): GaxiosOptions {
opts.headers = opts.headers || {};
if (!isBrowser()) {
if (typeof window === 'undefined') {
// set transporter user agent if not in browser
const uaValue: string = opts.headers['User-Agent'];
if (!uaValue) {
Expand Down
1 change: 0 additions & 1 deletion system-test/test.kitchen.ts
Expand Up @@ -15,7 +15,6 @@
*/

import * as assert from 'assert';
import * as cp from 'child_process';
import * as execa from 'execa';
import * as fs from 'fs';
import * as mv from 'mv';
Expand Down
1 change: 0 additions & 1 deletion test/test.compute.ts
Expand Up @@ -20,7 +20,6 @@ import {BASE_PATH, HEADERS, HOST_ADDRESS} from 'gcp-metadata';
import * as nock from 'nock';
import * as sinon from 'sinon';
import {Compute} from '../src';
import * as qs from 'querystring';

nock.disableNetConnect();

Expand Down
1 change: 0 additions & 1 deletion test/test.crypto.ts
@@ -1,4 +1,3 @@
import * as nativeCrypto from 'crypto';
import * as fs from 'fs';
import {assert} from 'chai';
import {createCrypto} from '../src/crypto/crypto';
Expand Down

0 comments on commit 83a5ba5

Please sign in to comment.