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

Make our code base more safe with Typescript more strict #14

Merged
merged 1 commit into from
Aug 29, 2019
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
src/generated/*
36 changes: 36 additions & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{
"env": {
"es6": true,
"node": true
},
"extends": [
"plugin:@typescript-eslint/recommended"
],
"parser": "@typescript-eslint/parser",
"plugins": ["@typescript-eslint"],
"globals": {
"Atomics": "readonly",
"SharedArrayBuffer": "readonly"
},
"parserOptions": {
"ecmaVersion": 2018,
"sourceType": "module",
"project": "./tsconfig.json",
"tsconfigRootDir": "./"
},
"rules": {
"indent": ["error", 2],
"max-len": ["error", 150],
"linebreak-style": ["error", "unix"],
"quotes": ["error", "single"],
"semi": ["error", "always"],
"object-curly-spacing": ["error", "always"],
"@typescript-eslint/no-explicit-any": "off",
"@typescript-eslint/no-empty-interface": "off",
"@typescript-eslint/explicit-function-return-type": ["error", {
"allowExpressions": true,
"allowTypedFunctionExpressions": true,
"allowHigherOrderFunctions": true
}]
}
}
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
node_modules/
dist/
yarn-error.log
yarn-error.log
.idea/
.vscode/
17 changes: 17 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,2 +1,19 @@
# nimbus-graphql
A GraphQL Framework with batteries included

# Developing

## VSCode users

Since `tslint` is being deprecated, we are using `eslint` to make this VSCode extension report issues on typescript files we need to add a setting.
To do that, add the following snippet on `settings.json`:
```json
{
"eslint.validate": [
"javascript",
"javascriptreact",
"typescript",
"typescriptreact"
]
}
```
23 changes: 14 additions & 9 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
"author": "Utilitywarehouse",
"license": "ISC",
"scripts": {
"build": "rm -rf ./dist && tsc -p tsconfig.json",
"build": "rm -rf ./dist && tsc --outDir ./dist -d -p ./",
"format": "prettier --write \"src/**/*.ts\"",
"lint": "tslint -p tsconfig.json -c tslint.json",
"lint:fix": "tslint -p tsconfig.json -c tslint.json --fix",
"lint": "eslint 'src/**/*.ts'",
"lint:fix": "eslint 'src/**/*.ts' --fix",
"test": "jest",
"test:watch": "jest --watch",
"test:cov": "jest --coverage",
Expand Down Expand Up @@ -48,21 +48,26 @@
"@types/bunyan": "^1.8.5",
"@types/google-protobuf": "^3.2.7",
"@types/graphql": "^14.0.3",
"@types/jest": "^23.3.10",
"@types/node": "^10.7.1",
"@types/graphql-iso-date": "^3.3.3",
"@types/graphql-type-json": "^0.3.2",
"@types/jest": "^24.0.18",
"@types/lodash": "^4.14.137",
"@types/node": "^12.7.2",
"@types/node-fetch": "^2.1.4",
"@types/protobufjs": "^6.0.0",
"@types/supertest": "^2.0.7",
"@types/uuid": "^3.4.4",
"@typescript-eslint/eslint-plugin": "^2.0.0",
"@typescript-eslint/parser": "^2.0.0",
"eslint": "^6.2.2",
"glob": "^7.1.3",
"grpc-tools": "^1.6.6",
"grpc-tools": "^1.8.0",
"grpc_tools_node_protoc_ts": "^2.4.2",
"jest": "^23.6.0",
"mockttp": "^0.12.4",
"prettier": "^1.14.2",
"supertest": "^3.4.2",
"ts-jest": "^23.10.5",
"ts-node": "^7.0.1",
"tsconfig-paths": "^3.7.0",
"tslint": "5.11.0"
"tsconfig-paths": "^3.7.0"
}
}
32 changes: 16 additions & 16 deletions src/__tests__/resolver.decorator.test.ts
Original file line number Diff line number Diff line change
@@ -1,71 +1,71 @@
import { After, Arg, Before, Context, Mutation, Parent, Query, Resolve, Resolver } from '../resolver.decorators';
import { ResolverRegistry } from '../resolver.registry';
import { makeExecutableSchema } from 'graphql-tools';
import { graphql } from 'graphql';
import { graphql, GraphQLSchema } from 'graphql';
import * as assert from 'assert';
import { NullError } from '../errors';

@Resolver('Decorator')
class ResolverClass {

id(parent) {
id(parent: any): string {
return parent.id + '-1';
}

@Before(() => {
throw new NullError('tried to access protected field');
throw new NullError('tried to access protected field');
})
@Query()
itsAlwaysNull() {
itsAlwaysNull(): string {
return 'you\'ll never see this message';
}

@Before(() => {
throw new Error('You hit before hook');
})
@Query()
beforeHook() {
beforeHook(): string {
return 'You\'ll never hit this';
}

@After(() => {
throw new Error('You hit after hook');
})
@Query()
afterHook() {
afterHook(): string {
return 'you\'ll never see this message';
}

@Resolve('OtherType.id')
field2(parent) {
field2(parent: any): string {
return parent.id + '-1';
}

@Query()
decorator() {
decorator(): any {
return { id: 'id-field' };
}

@Query('otherQuery')
other() {
other(): any {
return { id: 'id-other' };
}

@Mutation()
mutation() {
mutation(): string {
return 'mutated';
}

@Mutation('otherMutation')
mutation2() {
mutation2(): string {
return 'otherMutation';
}

methodWithMappedParams(
@Parent parent,
@Context('ctx') context,
@Arg('arg') arg,
) {
@Parent parent: any,
@Context('ctx') context: any,
@Arg('arg') arg: any,
): string {

assert(parent, 'Parent not present');
assert(context, 'Context not present');
Expand All @@ -77,7 +77,7 @@ class ResolverClass {

describe.only('GraphQL Decorators', () => {

let schema;
let schema: GraphQLSchema;
beforeEach(() => {
const schemaSDL = `
type Decorator {
Expand Down
10 changes: 5 additions & 5 deletions src/config/__tests__/coercion.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import {StringBoolean} from '../index';
import { StringBoolean } from '../index';

test('coercion', () => {
expect(StringBoolean('true')).toBe(true);
expect(StringBoolean('false')).toBe(false);
expect(StringBoolean('whatever')).toBe(false);
expect(StringBoolean('TRUE')).toBe(true);
expect(StringBoolean('true')).toBe(true);
expect(StringBoolean('false')).toBe(false);
expect(StringBoolean('whatever')).toBe(false);
expect(StringBoolean('TRUE')).toBe(true);
});
6 changes: 3 additions & 3 deletions src/config/__tests__/fixtures/example.default.config.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export default {
test: 'default-value',
other: 'other-value',
nan: Number('az'),
test: 'default-value',
other: 'other-value',
nan: Number('az'),
};
8 changes: 4 additions & 4 deletions src/config/__tests__/fixtures/example.section.config.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {Section} from '../../config.service';
import { Section } from '../../config.service';

export default {
[Section]: 'test',
test: 'section-value',
nan: Number('az'),
[Section]: 'test',
test: 'section-value',
nan: Number('az'),
};
94 changes: 47 additions & 47 deletions src/config/__tests__/service.test.ts
Original file line number Diff line number Diff line change
@@ -1,50 +1,50 @@
import {Config} from '../config.service';
import { Config } from '../config.service';

describe('config.service', () => {

let config;

beforeAll(async () => {
config = await Config.load(__dirname, 'fixtures/*.config.ts');
});

test('getting default value', () => {
expect(config.get('test')).toBe('default-value');
});

test('getting section value', () => {
expect(config.section('test').get('test')).toBe('section-value');
});

test('getting section value with fallback', () => {
expect(config.section('test').get('other')).toBe('other-value');
});

test('exception on unknown section key', () => {
expect(() => {
config.section('test').get('n-a');
}).toThrow('could not find requested key');
});

test('exception on unknown default key', () => {
expect(() => {
config.get('n-a');
}).toThrow('could not find requested key');
});

test('exception on NaN section value (bad coercion)', () => {
expect(() => {
config.section('test').get('nan');
}).toThrow('NaN encountered for key');
});

test('exception on NAN default value (bad coercion)', () => {
expect(() => {
config.get('nan');
}).toThrow('NaN encountered for key');
});

test('reverting to default on unknown section', () => {
expect(config.section('n-a').get('test')).toBe('default-value');
});
})
let config: Config;

beforeAll(() => {
config = Config.load(__dirname, 'fixtures/*.config.ts');
});

test('getting default value', () => {
expect(config.get('test')).toBe('default-value');
});

test('getting section value', () => {
expect(config.section('test').get('test')).toBe('section-value');
});

test('getting section value with fallback', () => {
expect(config.section('test').get('other')).toBe('other-value');
});

test('exception on unknown section key', () => {
expect(() => {
config.section('test').get('n-a');
}).toThrow('could not find requested key');
});

test('exception on unknown default key', () => {
expect(() => {
config.get('n-a');
}).toThrow('could not find requested key');
});

test('exception on NaN section value (bad coercion)', () => {
expect(() => {
config.section('test').get('nan');
}).toThrow('NaN encountered for key');
});

test('exception on NAN default value (bad coercion)', () => {
expect(() => {
config.get('nan');
}).toThrow('NaN encountered for key');
});

test('reverting to default on unknown section', () => {
expect(config.section('n-a').get('test')).toBe('default-value');
});
});
45 changes: 24 additions & 21 deletions src/config/config.getter.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,35 @@
import {ConfigProvider} from './config.provider';
import {ConfigContainer} from './config.container';
import { ConfigProvider } from './config.provider';
import { ConfigContainer } from './config.container';

export class Getter implements ConfigProvider {
constructor(
private readonly name: string,
private readonly section: ConfigContainer = {},
private readonly defaults: ConfigContainer = {}) {
}
constructor(
private readonly name: string,
private readonly section: ConfigContainer = {},
private readonly defaults: ConfigContainer = {}) {
}

get(key: string): any {
get(key: string): any {

let value;
let value;

if (this.section.hasOwnProperty(key)) {
value = this.section[key];
}
const sectionHasProperty = Object.prototype.hasOwnProperty.call(this.section, key);
const defaultsHasProperty = Object.prototype.hasOwnProperty.call(this.defaults, key);

if (!this.section.hasOwnProperty(key) && this.defaults.hasOwnProperty(key)) {
value = this.defaults[key];
}
if (sectionHasProperty) {
value = this.section[key];
}

if (Number.isNaN(value)) {
throw new Error(`NaN encountered for key '${key}' for section '${this.name}' in config`);
} else if (value === undefined) {
throw new Error(`could not find requested key '${key}' for section '${this.name}' in config`);
}
if (!sectionHasProperty && defaultsHasProperty) {
value = this.defaults[key];
}

return value;
if (Number.isNaN(value)) {
throw new Error(`NaN encountered for key '${key}' for section '${this.name}' in config`);
} else if (value === undefined) {
throw new Error(`could not find requested key '${key}' for section '${this.name}' in config`);
}

return value;
}

}
Loading