-
Notifications
You must be signed in to change notification settings - Fork 944
Enable strictBindCallApply. #2074
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
Changes from all commits
c581a3b
1fc55be
31dac0d
b0f6e83
928966e
54b14b0
d2c200e
f746285
90f7de0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -326,14 +326,15 @@ export class AsyncQueue { | |
`Attempted to schedule multiple operations with timer id ${timerId}.` | ||
); | ||
|
||
const delayedOp = DelayedOperation.createAndSchedule<unknown>( | ||
const delayedOp = DelayedOperation.createAndSchedule<T>( | ||
this, | ||
timerId, | ||
delayMs, | ||
op, | ||
op => this.removeDelayedOperation(op) | ||
removedOp => | ||
this.removeDelayedOperation(removedOp as DelayedOperation<unknown>) | ||
); | ||
this.delayedOperations.push(delayedOp); | ||
this.delayedOperations.push(delayedOp as DelayedOperation<unknown>); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without this you get:
for the |
||
|
||
return delayedOp; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,7 +42,7 @@ describeFn('WebChannel', () => { | |
const makeUrl = conn.makeUrl.bind(conn); | ||
|
||
it('includes project ID and database ID', () => { | ||
const url = makeUrl('Commit', {}); | ||
const url = makeUrl('Commit'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was just a bogus extra parameter. |
||
expect(url).to.equal( | ||
'http://example.com/v1/projects/testproject/' + | ||
'databases/(default)/documents:commit' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -89,15 +89,11 @@ export function isRunningAgainstEmulator(): boolean { | |
} | ||
|
||
/** | ||
* A wrapper around Jasmine's describe method that allows for it to be run with | ||
* A wrapper around Mocha's describe method that allows for it to be run with | ||
* persistence both disabled and enabled (if the browser is supported). | ||
*/ | ||
export const apiDescribe = apiDescribeInternal.bind(null, describe); | ||
apiDescribe.skip = apiDescribeInternal.bind(null, describe.skip); | ||
apiDescribe.only = apiDescribeInternal.bind(null, describe.only); | ||
|
||
function apiDescribeInternal( | ||
describeFn: Mocha.IContextDefinition, | ||
describeFn: Mocha.PendingSuiteFunction, | ||
message: string, | ||
testSuite: (persistence: boolean) => void | ||
): void { | ||
|
@@ -111,6 +107,23 @@ function apiDescribeInternal( | |
} | ||
} | ||
|
||
type ApiSuiteFunction = ( | ||
message: string, | ||
testSuite: (persistence: boolean) => void | ||
) => void; | ||
interface ApiDescribe { | ||
(message: string, testSuite: (persistence: boolean) => void): void; | ||
skip: ApiSuiteFunction; | ||
only: ApiSuiteFunction; | ||
} | ||
|
||
export const apiDescribe = apiDescribeInternal.bind( | ||
null, | ||
describe | ||
) as ApiDescribe; | ||
apiDescribe.skip = apiDescribeInternal.bind(null, describe.skip); | ||
apiDescribe.only = apiDescribeInternal.bind(null, describe.only); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without these changes, we were getting two errors:
For passing
for assigning to |
||
/** Converts the documents in a QuerySnapshot to an array with the data of each document. */ | ||
export function toDataArray( | ||
docSet: firestore.QuerySnapshot | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -87,9 +87,8 @@ export function addEqualityMatcher(): void { | |
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type | ||
const assertEql = (_super: (r: unknown, l: unknown) => boolean) => { | ||
originalFunction = originalFunction || _super; | ||
return function(this: unknown, ...args: unknown[]): void { | ||
return function(this: unknown, expected?: unknown, msg?: unknown): void { | ||
if (isActive) { | ||
const [expected, msg] = args; | ||
utils.flag(this, 'message', msg); | ||
const actual = utils.flag(this, 'object'); | ||
|
||
|
@@ -106,7 +105,7 @@ export function addEqualityMatcher(): void { | |
/*showDiff=*/ true | ||
); | ||
} else if (originalFunction) { | ||
originalFunction.apply(this, args); | ||
originalFunction.call(this, expected, msg); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without these changes we were getting:
|
||
} | ||
}; | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I split this into two overloads. One for collections with keys and values, one for collections with just keys. Without this, you get:
from the
f.call(this, r, s)
line below (fors
being undefined).