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
Enable strict mode in tsconfig and fix type errors #11200
Changes from 13 commits
aca904f
0da3d3d
78ccf61
69d528a
e503cbb
0fe49a0
e34d084
962bd47
36129f8
cea054e
bed3439
b568689
6498326
b8c6eb6
c3a761d
9eb5e3e
0966cc9
a101977
55a6351
f909921
e1e2fbb
0426c84
be151b0
c83b042
4aa485b
1e7c5c6
c080309
0b2cf25
9303630
5e3fb70
04dcc0d
2cb5c83
94b07f9
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 |
---|---|---|
|
@@ -6,6 +6,7 @@ import type { | |
} from "graphql"; | ||
import { visit } from "graphql"; | ||
|
||
import type { OptimisticWrapperFunction } from "optimism"; | ||
import { wrap } from "optimism"; | ||
|
||
import type { FragmentMap } from "../../utilities/index.js"; | ||
|
@@ -29,24 +30,22 @@ export function createFragmentRegistry( | |
return new FragmentRegistry(...fragments); | ||
} | ||
|
||
const { forEach: arrayLikeForEach } = Array.prototype; | ||
|
||
class FragmentRegistry implements FragmentRegistryAPI { | ||
private registry: FragmentMap = Object.create(null); | ||
|
||
// Call static method FragmentRegistry.from(...) instead of invoking the | ||
// Call `createFragmentRegistry` instead of invoking the | ||
// FragmentRegistry constructor directly. This reserves the constructor for | ||
// future configuration of the FragmentRegistry. | ||
constructor(...fragments: DocumentNode[]) { | ||
this.resetCaches(); | ||
if (fragments.length) { | ||
this.register.apply(this, fragments); | ||
this.register(...fragments); | ||
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 transpiles to the old code, but if we switch to a newer target we save a few bytes. |
||
} | ||
} | ||
|
||
public register(): this { | ||
public register(...fragments: DocumentNode[]): this { | ||
const definitions = new Map<string, FragmentDefinitionNode>(); | ||
arrayLikeForEach.call(arguments, (doc: DocumentNode) => { | ||
fragments.forEach((doc: DocumentNode) => { | ||
Comment on lines
+46
to
+48
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. 2384d2383
< var arrayLikeForEach = Array.prototype.forEach;
2398a2398,2401
> var fragments = [];
> for (var _i = 0; _i < arguments.length; _i++) {
> fragments[_i] = arguments[_i];
> }
2400c2403
< arrayLikeForEach.call(arguments, function (doc) {
---
> fragments.forEach(function (doc) { This transpiles slightly longer, are we okay with that? 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. I'm ok with it. |
||
getFragmentDefinitions(doc).forEach((node) => { | ||
definitions.set(node.name.value, node); | ||
}); | ||
|
@@ -66,27 +65,15 @@ class FragmentRegistry implements FragmentRegistryAPI { | |
private invalidate(name: string) {} | ||
|
||
public resetCaches() { | ||
this.invalidate = (this.lookup = this.cacheUnaryMethod("lookup")).dirty; // This dirty function is bound to the wrapped lookup method. | ||
this.transform = this.cacheUnaryMethod("transform"); | ||
this.findFragmentSpreads = this.cacheUnaryMethod("findFragmentSpreads"); | ||
this.invalidate = (this.lookup = this.cacheUnaryMethod(this.lookup)).dirty; // This dirty function is bound to the wrapped lookup method. | ||
this.transform = this.cacheUnaryMethod(this.transform); | ||
this.findFragmentSpreads = this.cacheUnaryMethod(this.findFragmentSpreads); | ||
} | ||
|
||
private cacheUnaryMethod< | ||
TName extends keyof Pick< | ||
FragmentRegistry, | ||
"lookup" | "transform" | "findFragmentSpreads" | ||
>, | ||
>(name: TName) { | ||
const registry = this; | ||
const originalMethod = FragmentRegistry.prototype[name]; | ||
return wrap( | ||
function () { | ||
return originalMethod.apply(registry, arguments); | ||
}, | ||
{ | ||
makeCacheKey: (arg) => arg, | ||
} | ||
); | ||
private cacheUnaryMethod<F extends (arg: any) => any>(originalMethod: F) { | ||
return wrap<Parameters<F>, ReturnType<F>>(originalMethod.bind(this), { | ||
makeCacheKey: (arg) => arg, | ||
}) as OptimisticWrapperFunction<Parameters<F>, ReturnType<F>> & F; | ||
} | ||
|
||
public lookup(fragmentName: string): FragmentDefinitionNode | null { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,18 +33,18 @@ type BroadcastOptions = Pick< | |
>; | ||
|
||
export class InMemoryCache extends ApolloCache<NormalizedCacheObject> { | ||
private data: EntityStore; | ||
private optimisticData: EntityStore; | ||
private data!: EntityStore; | ||
private optimisticData!: EntityStore; | ||
|
||
protected config: InMemoryCacheConfig; | ||
private watches = new Set<Cache.WatchOptions>(); | ||
private addTypename: boolean; | ||
|
||
private storeReader: StoreReader; | ||
private storeWriter: StoreWriter; | ||
private storeReader!: StoreReader; | ||
private storeWriter!: StoreWriter; | ||
private addTypenameTransform = new DocumentTransform(addTypenameToDocument); | ||
|
||
private maybeBroadcastWatch: OptimisticWrapperFunction< | ||
private maybeBroadcastWatch!: OptimisticWrapperFunction< | ||
Comment on lines
+36
to
+47
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. All of these are set in functions that are directly called from the constructor. |
||
[Cache.WatchOptions, BroadcastOptions?], | ||
any, | ||
[Cache.WatchOptions] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,6 +50,7 @@ function wrapDestructiveCacheMethod( | |
// that matters in any conceivable practical scenario. | ||
(destructiveMethodCounts.get(cache)! + 1) % 1e15 | ||
); | ||
// @ts-expect-error this is just too generic to be typed correctly | ||
return original.apply(this, arguments); | ||
}; | ||
} | ||
|
@@ -113,7 +114,7 @@ export class QueryInfo { | |
// NetworkStatus.loading, but also possibly fetchMore, poll, refetch, | ||
// or setVariables. | ||
networkStatus?: NetworkStatus; | ||
observableQuery?: ObservableQuery<any>; | ||
observableQuery?: ObservableQuery<any, any>; | ||
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. We are generally very inconsistent with the |
||
lastRequestId?: number; | ||
}): this { | ||
let networkStatus = query.networkStatus || NetworkStatus.loading; | ||
|
@@ -214,10 +215,10 @@ export class QueryInfo { | |
} | ||
} | ||
|
||
public readonly observableQuery: ObservableQuery<any> | null = null; | ||
public readonly observableQuery: ObservableQuery<any, any> | null = null; | ||
private oqListener?: QueryListener; | ||
|
||
setObservableQuery(oq: ObservableQuery<any> | null) { | ||
setObservableQuery(oq: ObservableQuery<any, any> | null) { | ||
if (oq === this.observableQuery) return; | ||
|
||
if (this.oqListener) { | ||
|
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.
This minifies even shorter than the old code.