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.

Sign up for GitHub

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

Jump to bottom

fix: promises where not allowed by type #61

Merged
merged 4 commits into from
Mar 1, 2023
Merged

fix: promises where not allowed by type #61

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b9ce0c8
fix: promises where not allowed by type
jspears Mar 1, 2023
02c9061
fix: added test
jspears Mar 1, 2023
4f43a74
removed test:types as it is no longer needed nor working
jspears Mar 1, 2023
93998ab
improved error message
jspears Mar 1, 2023
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
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"tdd": "mocha --watch",
"test": "npm run lint && npm run test:once && npm run test:types",
"test:once": "nyc --reporter=lcovonly --reporter=html mocha",
"test:types": "dtslint --expectOnly --localTs node_modules/typescript/lib types",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this didn't work before I made changes, kept returning

> dtslint --expectOnly --localTs node_modules/typescript/lib types

Error: Errors in typescript@local for external dependencies:
../src/Schema.ts(33,37): error TS2345: Argument of type 'DataType' is not assignable to parameter of type 'PlainObject<any>'.
../src/Schema.ts(48,37): error TS2345: Argument of type 'DataType' is not assignable to parameter of type 'PlainObject<any>'.

    at /Users/jspears/dev/schema-typed/node_modules/dtslint/bin/index.js:207:19
    at Generator.next (<anonymous>)
    at fulfilled (/Users/jspears/dev/schema-typed/node_modules/dtslint/bin/index.js:6:58)

Copy link
Member

Choose a reason for hiding this comment

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

dtslint is a linting tool used by the current library before refactoring through typescript, I think it can be removed, because there is eslint as a type checking tool.

"test:types": "#dtslint --expectOnly --localTs node_modules/typescript/lib types",
"doctoc:": "doctoc README.md",
"changelog": "conventional-changelog -p angular -i CHANGELOG.md -s"
},
Expand Down
2 changes: 1 addition & 1 deletion src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export type ValidCallbackType<V, D, E> = (
value: V,
data?: D,
filedName?: string | string[]
) => CheckResult<E> | boolean;
) => CheckResult<E> | boolean | Promise<boolean | CheckResult<E>>;
export type PlainObject<T extends Record<string, unknown> = any> = {
[P in keyof T]: T;
};
Expand Down
9 changes: 8 additions & 1 deletion src/utils/createValidator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { CheckResult, RuleType } from '../types';
import formatErrorMessage from './formatErrorMessage';

function isObj(o: unknown): o is Record<PropertyKey, unknown> {
return o != null && (typeof o === 'object' || typeof o == 'function');
}
function isPromiseLike(v: unknown): v is Promise<unknown> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

sometimes people don't use real promises. Its sad, but true.

Copy link
Member

Choose a reason for hiding this comment

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

👌

return v instanceof Promise || (isObj(v) && typeof v.then === 'function');
}
/**
* Create a data validator
* @param data
Expand All @@ -19,6 +24,8 @@ export function createValidator<V, D, E>(data?: D, name?: string | string[]) {
name: Array.isArray(name) ? name.join('.') : name
})
};
} else if (isPromiseLike(checkResult)) {
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to be able to add test cases for the changes here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point added a test.

throw new Error('synchronous validator had an async result');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems like the safest thing to do. I would hate for somebody to make an async call that never resolves.

} else if (typeof checkResult === 'object' && (checkResult.hasError || checkResult.array)) {
return checkResult;
}
Expand Down