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

enable typescript strict mode #3010

Merged
merged 57 commits into from
Jun 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
c10d89e
enable noImplicitThis
n1ru4l May 27, 2021
58e1f25
enable `strictBindCallApply`
n1ru4l May 27, 2021
0099821
enable `strictFunctionTypes` 1: testing
n1ru4l May 27, 2021
869a6df
enable `strictFunctionTypes` 2: utils
n1ru4l May 27, 2021
2bac22d
enable strictFunctionTypes 3: schema
n1ru4l May 27, 2021
a400227
enable strictFunctionTypes 4: batch-execute
n1ru4l May 27, 2021
072e9ec
enable strictFunctionTypes 5 `load`
n1ru4l May 27, 2021
ed58bc8
enable strictFunctionTypes 6 `loaders/url`
n1ru4l May 27, 2021
71f40c7
enable strictFunctionTypes 7 `webpack-loader`
n1ru4l May 27, 2021
d1bd5f8
enable strictFunctionTypes 8 `links`
n1ru4l May 27, 2021
63dc88c
enable strictFunctionTypes 9 `delegate` and `batch-delegate`
n1ru4l May 27, 2021
c54e8d0
enable `strictFunctionTypes` 10 `wrap`
n1ru4l May 27, 2021
fb67464
enable `strictFunctionTypes` 11 `stitch` and `stitching-directive`
n1ru4l May 27, 2021
acc6263
enable `strictFunctionTypes` final tsconfig
n1ru4l May 27, 2021
d8bdfef
remove unnecessary generic params
n1ru4l May 27, 2021
40426ef
enable `strictNullChecks` 1 `utils`
n1ru4l May 28, 2021
01145bd
enable `strictNullChecks` 2 `schema`
n1ru4l May 28, 2021
673d62d
enable `strictNullChecks` 3 `resolvers-composition`
n1ru4l May 28, 2021
e6f88cb
enable `strictNullChecks` 4 `webpack-loader`
n1ru4l May 28, 2021
61a2c22
enable `strictNullChecks` 5 `batch-execute`
n1ru4l May 28, 2021
62c3ae9
enable `strictNullChecks` 6 `graphql-tag-plug`
n1ru4l May 28, 2021
bef41e4
enable `strictNullChecks` 7 `import`
n1ru4l May 28, 2021
e1afdb0
enable `strictNullChecks` 8 `loaders/url`
n1ru4l May 28, 2021
18091cc
enable `strictNullChecks` 8 `loaders/prisma`
n1ru4l May 28, 2021
604dcf7
enable `strictNullChecks` 8 `loaders/module`
n1ru4l May 28, 2021
3f00108
enable `strictNullChecks` 9 `loaders/github`
n1ru4l May 28, 2021
451df99
enable `strictNullChecks` 12 `loaders/code-file`
n1ru4l May 28, 2021
225dc5c
enable `strictNullChecks` 13 `load`
n1ru4l May 28, 2021
7fd2468
enable `strictNullChecks` 14 `delegate`
n1ru4l Jun 3, 2021
b8d392b
enable `strictNullChecks` 15 `batch-delegate`
n1ru4l Jun 3, 2021
f09afd1
interlude: move commonly used stuff to testing folder
n1ru4l Jun 3, 2021
78b7d53
enable `strictNullChecks` 16 `wrap`
n1ru4l Jun 3, 2021
ab7dff5
enable `strictNullChecks` 17 `stitch`
n1ru4l Jun 3, 2021
c2b86c8
enable `strictNullChecks` 18 `load-files`
n1ru4l Jun 3, 2021
48da64a
enable `strictNullChecks` 19 `load-typedefs`
n1ru4l Jun 3, 2021
25bb0de
enable `strictNullChecks` 20 `merge`
n1ru4l Jun 4, 2021
b870c88
enable `strictNullChecks` 21 `node-require`
n1ru4l Jun 4, 2021
2af1ba3
enable `strictNullChecks` 22 `mock`
n1ru4l Jun 4, 2021
04d4ff4
enable `strictNullChecks` 22 `stitching-directive`
n1ru4l Jun 4, 2021
4106e68
enable `strictNullChecks` flag in tsconfig
n1ru4l Jun 4, 2021
e6c9433
fix: graphql-js 14 compat
n1ru4l Jun 4, 2021
51f13f6
fix `strictPropertyInitialization` errors
n1ru4l Jun 4, 2021
13edfdd
enable strictPropertyInitialization rule
n1ru4l Jun 4, 2021
8bd511e
strict mode!!!
n1ru4l Jun 4, 2021
d00ada6
enable `noPropertyAccessFromIndexSignature`
n1ru4l Jun 4, 2021
81ffd42
enable `noFallthroughCasesInSwitch` and `noPropertyAccessFromIndexSig…
n1ru4l Jun 4, 2021
39be50c
Add strict to new changes
ardatan Jun 8, 2021
8249e56
fix: remove casts as upstream issue is fixed
n1ru4l Jun 10, 2021
9b42a69
fix: throw if we cannot identify the operation type.
n1ru4l Jun 10, 2021
e6fd004
fix: change wording of error
n1ru4l Jun 10, 2021
78f7012
fix: make error message more clear
n1ru4l Jun 10, 2021
4e6c9ae
refactor: use null/undefined instead of falsey check
n1ru4l Jun 10, 2021
1394099
fix: assert operation from document
n1ru4l Jun 10, 2021
5800b78
fix: type narrowing not working due to generics
n1ru4l Jun 10, 2021
3a6a64b
fix: apply feedback for relocateError
n1ru4l Jun 10, 2021
453ceb3
Make wrapSchema generic. Fix #3064 (#1)
AugustinLF Jun 11, 2021
29ea0a7
Fix Build
ardatan Jun 21, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 22 additions & 6 deletions packages/batch-delegate/src/getLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,34 @@ import { BatchDelegateOptions } from './types';

const cache1: WeakMap<
ReadonlyArray<FieldNode>,
WeakMap<GraphQLSchema | SubschemaConfig, Record<string, DataLoader<any, any>>>
WeakMap<GraphQLSchema | SubschemaConfig<any, any, any, any>, Record<string, DataLoader<any, any>>>
> = new WeakMap();

function createBatchFn<K = any>(options: BatchDelegateOptions) {
const argsFromKeys = options.argsFromKeys ?? ((keys: ReadonlyArray<K>) => ({ ids: keys }));
const fieldName = options.fieldName ?? options.info.fieldName;
const { valuesFromResults, lazyOptionsFn } = options;

return async (keys: ReadonlyArray<K>) => {
const results = await delegateToSchema({
returnType: new GraphQLList(getNamedType(options.info.returnType) as GraphQLOutputType),
onLocatedError: originalError =>
relocatedError(originalError, originalError.path.slice(0, 0).concat(originalError.path.slice(2))),
Copy link
Collaborator

Choose a reason for hiding this comment

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

The relocatedError shim removes the batched index when merging errors into results, and callers should receive the same behavior as if they had not batched.
If there is no path, relocatedError(originalError, undefined) should cause the error to be copied, but it is unnecessary, and we could just return the originalError. For correctness, we could/should also check to make sure that the first element of the path is the fieldName and the second is a number, because some servers do not properly path errors.

Copy link
Collaborator Author

@n1ru4l n1ru4l Jun 10, 2021

Choose a reason for hiding this comment

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

Please let me know whether bef2164 is what you had in mind

onLocatedError: originalError => {
if (originalError.path == null) {
return originalError;
}

const [pathFieldName, pathNumber] = originalError.path;

if (pathFieldName !== fieldName) {
throw new Error(`Error path value at index 0 should be '${fieldName}', received '${pathFieldName}'.`);
}
const pathNumberType = typeof pathNumber;
if (pathNumberType !== 'number') {
throw new Error(`Error path value at index 1 should be of type number, received '${pathNumberType}'.`);
}

return relocatedError(originalError, originalError.path.slice(0, 0).concat(originalError.path.slice(2)));
},
args: argsFromKeys(keys),
...(lazyOptionsFn == null ? options : lazyOptionsFn(options)),
});
Expand All @@ -35,10 +51,10 @@ function createBatchFn<K = any>(options: BatchDelegateOptions) {
};
}

export function getLoader<K = any, V = any, C = K>(options: BatchDelegateOptions): DataLoader<K, V, C> {
export function getLoader<K = any, V = any, C = K>(options: BatchDelegateOptions<any>): DataLoader<K, V, C> {
const fieldName = options.fieldName ?? options.info.fieldName;

let cache2: WeakMap<GraphQLSchema | SubschemaConfig, Record<string, DataLoader<K, V, C>>> = cache1.get(
let cache2: WeakMap<GraphQLSchema | SubschemaConfig, Record<string, DataLoader<K, V, C>>> | undefined = cache1.get(
options.info.fieldNodes
);

Expand All @@ -56,7 +72,7 @@ export function getLoader<K = any, V = any, C = K>(options: BatchDelegateOptions
let loaders = cache2.get(options.schema);

if (loaders === undefined) {
loaders = Object.create(null);
loaders = Object.create(null) as Record<string, DataLoader<K, V, C>>;
cache2.set(options.schema, loaders);
const batchFn = createBatchFn(options);
const loader = new DataLoader<K, V, C>(keys => batchFn(keys), options.dataLoaderOptions);
Expand Down
10 changes: 5 additions & 5 deletions packages/batch-delegate/tests/basic.example.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ describe('batch delegation within basic stitching example', () => {

expect(numCalls).toEqual(1);
expect(result.errors).toBeUndefined();
expect(result.data.trendingChirps[0].chirpedAtUser.email).not.toBe(null);
expect(result.data!.trendingChirps[0].chirpedAtUser.email).not.toBe(null);
});

test('works with key arrays', async () => {
Expand All @@ -108,9 +108,9 @@ describe('batch delegation within basic stitching example', () => {
`,
resolvers: {
Query: {
posts: (obj, args) => {
posts: (_, args) => {
numCalls += 1;
return args.ids.map(id => ({ id, title: `Post ${id}` }));
return args.ids.map((id: unknown) => ({ id, title: `Post ${id}` }));
}
}
}
Expand All @@ -129,8 +129,8 @@ describe('batch delegation within basic stitching example', () => {
`,
resolvers: {
Query: {
users: (obj, args) => {
return args.ids.map(id => {
users: (_, args) => {
return args.ids.map((id: unknown) => {
return { id, postIds: [Number(id)+1, Number(id)+2] };
});
}
Expand Down
2 changes: 1 addition & 1 deletion packages/batch-delegate/tests/withTransforms.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ describe('works with complex transforms', () => {
context,
info,
transforms: [queryTransform],
returnType: new GraphQLList(new GraphQLList(info.schema.getType('Book')))
returnType: new GraphQLList(new GraphQLList(info.schema.getType('Book')!))
}),
},
},
Expand Down
43 changes: 27 additions & 16 deletions packages/batch-execute/src/createBatchingExecutor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,37 @@ import { splitResult } from './splitResult';
export function createBatchingExecutor(
executor: Executor,
dataLoaderOptions?: DataLoader.Options<any, any, any>,
extensionsReducer?: (mergedExtensions: Record<string, any>, executionParams: ExecutionParams) => Record<string, any>
extensionsReducer: (
mergedExtensions: Record<string, any>,
executionParams: ExecutionParams
) => Record<string, any> = defaultExtensionsReducer
): Executor {
const loader = new DataLoader(
createLoadFn(executor, extensionsReducer ?? defaultExtensionsReducer),
dataLoaderOptions
);
const loader = new DataLoader(createLoadFn(executor, extensionsReducer), dataLoaderOptions);
return (executionParams: ExecutionParams) => loader.load(executionParams);
}

function createLoadFn(
executor: ({ document, context, variables, info }: ExecutionParams) => ExecutionResult | Promise<ExecutionResult>,
executor: Executor,
extensionsReducer: (mergedExtensions: Record<string, any>, executionParams: ExecutionParams) => Record<string, any>
) {
return async (execs: Array<ExecutionParams>): Promise<Array<ExecutionResult>> => {
return async (execs: ReadonlyArray<ExecutionParams>): Promise<Array<ExecutionResult>> => {
const execBatches: Array<Array<ExecutionParams>> = [];
let index = 0;
const exec = execs[index];
let currentBatch: Array<ExecutionParams> = [exec];
execBatches.push(currentBatch);
const operationType = getOperationAST(exec.document, undefined).operation;

const operationType = getOperationAST(exec.document, undefined)?.operation;
if (operationType == null) {
throw new Error('Could not identify operation type of document.');
}

while (++index < execs.length) {
const currentOperationType = getOperationAST(execs[index].document, undefined).operation;
const currentOperationType = getOperationAST(execs[index].document, undefined)?.operation;
if (operationType == null) {
throw new Error('Could not identify operation type of document.');
}

if (operationType === currentOperationType) {
currentBatch.push(execs[index]);
} else {
Expand All @@ -48,13 +57,15 @@ function createLoadFn(
executionResults.push(new ValueOrPromise(() => executor(mergedExecutionParams)));
});

return ValueOrPromise.all(executionResults).then(resultBatches => {
let results: Array<ExecutionResult> = [];
resultBatches.forEach((resultBatch, index) => {
results = results.concat(splitResult(resultBatch, execBatches[index].length));
});
return results;
}).resolve();
return ValueOrPromise.all(executionResults)
.then(resultBatches => {
let results: Array<ExecutionResult> = [];
resultBatches.forEach((resultBatch, index) => {
results = [...results, ...splitResult(resultBatch!, execBatches[index].length)];
});
return results;
})
.resolve();
};
}

Expand Down
6 changes: 4 additions & 2 deletions packages/batch-execute/src/getBatchingExecutor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ import { memoize2of4 } from './memoize';
export const getBatchingExecutor = memoize2of4(function (
_context: Record<string, any> = self ?? window ?? global,
executor: Executor,
dataLoaderOptions?: DataLoader.Options<any, any, any>,
extensionsReducer?: (mergedExtensions: Record<string, any>, executionParams: ExecutionParams) => Record<string, any>
dataLoaderOptions?: DataLoader.Options<any, any, any> | undefined,
extensionsReducer?:
| undefined
| ((mergedExtensions: Record<string, any>, executionParams: ExecutionParams) => Record<string, any>)
): Executor {
return createBatchingExecutor(executor, dataLoaderOptions, extensionsReducer);
});
17 changes: 11 additions & 6 deletions packages/batch-execute/src/mergeExecutionParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
OperationTypeNode,
} from 'graphql';

import { ExecutionParams } from '@graphql-tools/utils';
import { ExecutionParams, Maybe } from '@graphql-tools/utils';

import { createPrefix } from './prefix';

Expand Down Expand Up @@ -66,7 +66,7 @@ export function mergeExecutionParams(
const mergedFragmentDefinitions: Array<FragmentDefinitionNode> = [];
let mergedExtensions: Record<string, any> = Object.create(null);

let operation: OperationTypeNode;
let operation: Maybe<OperationTypeNode>;

execs.forEach((executionParams, index) => {
const prefixedExecutionParams = prefixExecutionParams(createPrefix(index), executionParams);
Expand All @@ -85,6 +85,10 @@ export function mergeExecutionParams(
mergedExtensions = extensionsReducer(mergedExtensions, executionParams);
});

if (operation == null) {
throw new Error('Could not identify operation type. Did the document only include fragment definitions?');
}

const mergedOperationDefinition: OperationDefinitionNode = {
kind: Kind.OPERATION_DEFINITION,
operation,
Expand All @@ -109,7 +113,8 @@ export function mergeExecutionParams(

function prefixExecutionParams(prefix: string, executionParams: ExecutionParams): ExecutionParams {
let document = aliasTopLevelFields(prefix, executionParams.document);
const variableNames = Object.keys(executionParams.variables);
const executionVariables = executionParams.variables ?? {};
const variableNames = Object.keys(executionVariables);

if (variableNames.length === 0) {
return { ...executionParams, document };
Expand All @@ -122,7 +127,7 @@ function prefixExecutionParams(prefix: string, executionParams: ExecutionParams)
});

const prefixedVariables = variableNames.reduce((acc, name) => {
acc[prefix + name] = executionParams.variables[name];
acc[prefix + name] = executionVariables[name];
return acc;
}, Object.create(null));

Expand Down Expand Up @@ -150,9 +155,9 @@ function aliasTopLevelFields(prefix: string, document: DocumentNode): DocumentNo
};
},
};
return visit(document, transformer, ({
return visit(document, transformer, {
[Kind.DOCUMENT]: [`definitions`],
} as unknown) as VisitorKeyMap<ASTKindToNode>);
} as unknown as VisitorKeyMap<ASTKindToNode>);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/batch-execute/src/prefix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ export function createPrefix(index: number): string {
return `graphqlTools${index}_`;
}

export function parseKey(prefixedKey: string): { index: number; originalKey: string } {
export function parseKey(prefixedKey: string): null | { index: number; originalKey: string } {
const match = /^graphqlTools([\d]+)_(.*)$/.exec(prefixedKey);
if (match && match.length === 3 && !isNaN(Number(match[1])) && match[2]) {
return { index: Number(match[1]), originalKey: match[2] };
Expand Down
16 changes: 11 additions & 5 deletions packages/batch-execute/src/splitResult.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import { ExecutionResult, GraphQLError } from 'graphql';

import { relocatedError } from '@graphql-tools/utils';
import { assertSome, relocatedError } from '@graphql-tools/utils';

import { parseKey } from './prefix';

Expand All @@ -18,11 +18,17 @@ export function splitResult(mergedResult: ExecutionResult, numResults: number):
const data = mergedResult.data;
if (data) {
Object.keys(data).forEach(prefixedKey => {
const { index, originalKey } = parseKey(prefixedKey);
if (!splitResults[index].data) {
splitResults[index].data = { [originalKey]: data[prefixedKey] };
const parsedKey = parseKey(prefixedKey);
assertSome(parsedKey, "'parsedKey' should not be null.");
const { index, originalKey } = parsedKey;
const result = splitResults[index];
if (result == null) {
return;
}
if (result.data == null) {
result.data = { [originalKey]: data[prefixedKey] };
} else {
splitResults[index].data[originalKey] = data[prefixedKey];
result.data[originalKey] = data[prefixedKey];
}
});
}
Expand Down
5 changes: 3 additions & 2 deletions packages/delegate/src/Subschema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ interface ISubschema<K = any, V = any, C = K, TContext = Record<string, any>>
}

export class Subschema<K = any, V = any, C = K, TContext = Record<string, any>>
implements ISubschema<K, V, C, TContext> {
implements ISubschema<K, V, C, TContext>
{
public schema: GraphQLSchema;

public rootValue?: Record<string, any>;
Expand All @@ -25,7 +26,7 @@ export class Subschema<K = any, V = any, C = K, TContext = Record<string, any>>
public batchingOptions?: BatchingOptions<K, V, C>;

public createProxyingResolver?: CreateProxyingResolverFn<TContext>;
public transforms: Array<Transform>;
public transforms: Array<Transform<any, TContext>>;
public transformedSchema: GraphQLSchema;

public merge?: Record<string, MergedTypeConfig<any, any, TContext>>;
Expand Down
6 changes: 3 additions & 3 deletions packages/delegate/src/Transformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ interface Transformation {
context: Record<string, any>;
}

export class Transformer {
export class Transformer<TContext = Record<string, any>> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is there any benefit in passing the TContext along with the Transforms or does it make more sense to just use any for all the type signatures that complain that TContext might be wrong (e.g. in the constructor)?

Copy link
Collaborator Author

@n1ru4l n1ru4l May 27, 2021

Choose a reason for hiding this comment

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

I guess yes as DelegationContext holds information about the Context?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think those who write transforms involving the context need that to be typed correctly and passed correctly to their transforms. I am not 100% on how our use of any will affect those users. :(

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could release an alpha and ask for feedback? But, this would slow down the process of actually releasing this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But since it is just a default value and everyone could overwrite it if desired I guess it is fine to leave it like that.

private transformations: Array<Transformation> = [];
private delegationContext: DelegationContext;
private delegationContext: DelegationContext<any>;

constructor(context: DelegationContext, binding: DelegationBinding = defaultDelegationBinding) {
constructor(context: DelegationContext<TContext>, binding: DelegationBinding<TContext> = defaultDelegationBinding) {
this.delegationContext = context;
const delegationTransforms: Array<Transform> = binding(this.delegationContext);
delegationTransforms.forEach(transform => this.addTransform(transform, {}));
Expand Down
6 changes: 3 additions & 3 deletions packages/delegate/src/applySchemaTransforms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ import { GraphQLSchema } from 'graphql';

import { cloneSchema } from '@graphql-tools/utils';

import { SubschemaConfig, Transform } from './types';
import { SubschemaConfig } from './types';

export function applySchemaTransforms(
originalWrappingSchema: GraphQLSchema,
subschemaConfig: SubschemaConfig,
subschemaConfig: SubschemaConfig<any, any, any, any>,
transformedSchema?: GraphQLSchema
): GraphQLSchema {
const schemaTransforms = subschemaConfig.transforms;
Expand All @@ -16,7 +16,7 @@ export function applySchemaTransforms(
}

return schemaTransforms.reduce(
(schema: GraphQLSchema, transform: Transform) =>
(schema: GraphQLSchema, transform) =>
transform.transformSchema != null
? transform.transformSchema(cloneSchema(schema), subschemaConfig, transformedSchema)
: schema,
Expand Down
Loading