Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build!: update to latest version of gts/typescript #183

Merged
merged 3 commits into from Mar 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .eslintrc.json
@@ -0,0 +1,3 @@
{
"extends": "./node_modules/gts"
}
15 changes: 0 additions & 15 deletions .eslintrc.yml

This file was deleted.

2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Expand Up @@ -9,7 +9,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
node: [8, 10, 12, 13]
node: [10, 12, 13]
steps:
- uses: actions/checkout@v1
- uses: actions/setup-node@v1
Expand Down
8 changes: 5 additions & 3 deletions .prettierignore
@@ -1,3 +1,5 @@
node_modules/*
samples/node_modules/*
src/**/doc/*
**/node_modules
**/.coverage
build/
docs/
protos/
8 changes: 0 additions & 8 deletions .prettierrc

This file was deleted.

17 changes: 17 additions & 0 deletions .prettierrc.js
@@ -0,0 +1,17 @@
// Copyright 2020 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

module.exports = {
...require('gts/.prettierrc.json')
}
7 changes: 4 additions & 3 deletions package.json
Expand Up @@ -33,13 +33,14 @@
"@types/node": "^10.5.2",
"@types/sinon": "^7.0.0",
"c8": "^7.0.0",
"chai": "^4.2.0",
"codecov": "^3.0.4",
"gts": "^1.0.0",
"gts": "^2.0.0-alpha.5",
"hard-rejection": "^2.1.0",
"linkinator": "^2.0.0",
"mocha": "^7.0.0",
"mocha": "^7.1.1",
"sinon": "^9.0.0",
"typescript": "3.6.4"
"typescript": "^3.8.3"
},
"engines": {
"node": ">=10"
Expand Down
4 changes: 4 additions & 0 deletions samples/package.json
Expand Up @@ -5,6 +5,10 @@
"engines": {
"node": ">=8"
},
"files": [
"*.js",
"!test/"
],
"repository": "googleapis/nodejs-promisify",
"private": true,
"scripts": {
Expand Down
2 changes: 1 addition & 1 deletion samples/test/test.js
Expand Up @@ -16,7 +16,7 @@ const {assert} = require('chai');
const {describe, it} = require('mocha');
const cp = require('child_process');

const execSync = (cmd) => cp.execSync(cmd, {encoding: 'utf-8'});
const execSync = cmd => cp.execSync(cmd, {encoding: 'utf-8'});

describe('quickstart samples', () => {
it('should run the quickstart', async () => {
Expand Down
21 changes: 10 additions & 11 deletions src/index.ts
@@ -1,3 +1,5 @@
/* eslint-disable prefer-rest-params */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should move this after the license header I think

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed the eslint-disable inline statements, we will probably want a replacement soonish depending on how migrating other libraries goes.


// Copyright 2014 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
Expand Down Expand Up @@ -66,8 +68,7 @@ export function promisify(
const slice = Array.prototype.slice;

// tslint:disable-next-line:no-any
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly these won't work with eslint. There's a different syntax (which I can't remember right now) for disabling a line.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screen Shot 2020-03-23 at 1 47 33 PM

Is this useful 👆

const wrapper: any = function(this: WithPromise) {
const context = this;
const wrapper: any = function (this: WithPromise) {
let last;

for (last = arguments.length - 1; last >= 0; last--) {
Expand All @@ -81,7 +82,7 @@ export function promisify(
break; // non-callback last argument found.
}

return originalMethod.apply(context, arguments);
return originalMethod.apply(this, arguments);
}

// peel trailing undefined.
Expand All @@ -93,8 +94,8 @@ export function promisify(
// Because dedupe will likely create a single install of
// @google-cloud/common to be shared amongst all modules, we need to
// localize it at the Service level.
if (context && context.Promise) {
PromiseCtor = context.Promise;
if (this && this.Promise) {
PromiseCtor = this.Promise;
}

return new PromiseCtor((resolve, reject) => {
Expand All @@ -114,7 +115,7 @@ export function promisify(
}
});

originalMethod.apply(context, args);
originalMethod.apply(this, args);
});
};

Expand Down Expand Up @@ -165,16 +166,14 @@ export function callbackify(originalMethod: CallbackMethod) {
}

// tslint:disable-next-line:no-any
const wrapper = function(this: any) {
const context = this;

const wrapper = function (this: any) {
if (typeof arguments[arguments.length - 1] !== 'function') {
return originalMethod.apply(context, arguments);
return originalMethod.apply(this, arguments);
}

const cb = Array.prototype.pop.call(arguments);

originalMethod.apply(context, arguments).then(
originalMethod.apply(this, arguments).then(
// tslint:disable-next-line:no-any
(res: any) => {
res = Array.isArray(res) ? res : [res];
Expand Down
32 changes: 14 additions & 18 deletions test/index.ts
Expand Up @@ -12,8 +12,10 @@
// See the License for the specific language governing permissions and
// limitations under the License.

/* eslint-disable @typescript-eslint/no-empty-function,prefer-rest-params */

import * as assert from 'assert';
import {describe, it, afterEach} from 'mocha';
import {describe, it, afterEach, beforeEach} from 'mocha';
import * as sinon from 'sinon';
import * as util from '../src';

Expand All @@ -24,13 +26,12 @@ describe('promisifyAll', () => {
const fakeArgs = [null, 1, 2, 3];
const fakeError = new Error('err.');

// tslint:disable-next-line
let FakeClass: any;

beforeEach(() => {
FakeClass = class {
methodName(callback: Function) {
callback.apply(null, fakeArgs);
callback(...fakeArgs);
}
methodSingle(callback: Function) {
callback(null, fakeArgs[1]);
Expand Down Expand Up @@ -83,7 +84,6 @@ describe('promisifyAll', () => {
});
`);
} catch (error) {
// tslint:disable-next-line ban
it.skip('should work on ES classes');
}

Expand Down Expand Up @@ -131,8 +131,7 @@ describe('promisify', () => {

beforeEach(() => {
fakeArgs = [null, 1, 2, 3];
func = util.promisify(function(this: {}, callback: () => void) {
// tslint:disable-next-line no-any
func = util.promisify(function (this: {}, callback: () => void) {
(callback as any).apply(this, fakeArgs);
});
});
Expand All @@ -144,12 +143,13 @@ describe('promisify', () => {
});

it('should not return a promise in callback mode', done => {
let returnVal: {};
returnVal = func.call(fakeContext, function(this: {}) {
const args = [].slice.call(arguments);
let returnVal: any;
returnVal = func.call(fakeContext, function (this: {}) {
const args = [...arguments];
assert.deepStrictEqual(args, fakeArgs);
assert.strictEqual(this, fakeContext);
assert(!returnVal);
returnVal = null; // this is to suppress prefer-const.
done();
});
});
Expand All @@ -174,7 +174,6 @@ describe('promisify', () => {
});

it('should allow the Promise object to be overridden', () => {
// tslint:disable-next-line:variable-name
const FakePromise = class {};
const promise = func.call({Promise: FakePromise});
assert(promise instanceof FakePromise);
Expand All @@ -185,7 +184,6 @@ describe('promisify', () => {

func = util.promisify(
(callback: () => void) => {
// tslint:disable-next-line no-any
(callback as any).apply(func, [null, fakeArg]);
},
{
Expand All @@ -199,7 +197,6 @@ describe('promisify', () => {
});

it('should ignore singular when multiple args are present', () => {
// tslint:disable-next-line:no-any
const fakeArgs: any[] = ['a', 'b'];

func = util.promisify(
Expand Down Expand Up @@ -259,7 +256,6 @@ describe('callbackifyAll', () => {
const fakeArgs = [1, 2, 3];
const fakeError = new Error('err.');

// tslint:disable-next-line
let FakeClass: any;

beforeEach(() => {
Expand Down Expand Up @@ -313,7 +309,7 @@ describe('callbackify', () => {
beforeEach(() => {
fakeArgs = [1, 2, 3];

func = util.callbackify(async function(this: {}) {
func = util.callbackify(async (_this: {}) => {
return fakeArgs;
});
});
Expand All @@ -331,15 +327,15 @@ describe('callbackify', () => {
});

it('should call the callback if it is provided', done => {
func(function(this: {}) {
func(function (this: {}) {
const args = [].slice.call(arguments);
assert.deepStrictEqual(args, [null, ...fakeArgs]);
done();
});
});

it('should call the provided callback with undefined', done => {
func = util.callbackify(async function(this: {}) {});
func = util.callbackify(async (_this: {}) => {});
func((err: Error, resp: {}) => {
assert.strictEqual(err, null);
assert.strictEqual(resp, undefined);
Expand All @@ -348,10 +344,10 @@ describe('callbackify', () => {
});

it('should call the provided callback with null', done => {
func = util.callbackify(async function(this: {}) {
func = util.callbackify(async (_this: {}) => {
return null;
});
func(function(this: {}) {
func(function (this: {}) {
const args = [].slice.call(arguments);
assert.deepStrictEqual(args, [null, null]);
done();
Expand Down
1 change: 1 addition & 0 deletions tsconfig.json
@@ -1,6 +1,7 @@
{
"extends": "./node_modules/gts/tsconfig-google.json",
"compilerOptions": {
"lib": ["es2018", "dom"],
"rootDir": ".",
"outDir": "build",
},
Expand Down