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

Add type assertions for JS users #16

Open
zpalmtree opened this issue May 10, 2019 · 0 comments

Comments

Projects
None yet
1 participant
@zpalmtree
Copy link
Collaborator

commented May 10, 2019

Currently you can pass incorrect types to functions without it throwing. This usually causes hard to diagnose errors. We should add assertions to check each type is as expected, and throw if not.

I think we have lodash as a dependency already, they might help with some of the type checks.

For example:

public async reset(scanHeight: number = 0, scanTimestamp: number = 0): Promise<void> {
    // ...
}

could be altered to (using https://lodash.com/docs/4.17.11#isNumber) (Ensure we've imported it in the file you're using first)

public async reset(scanHeight: number = 0, scanTimestamp: number = 0): Promise<void> {
    if (_.isNumber(scanHeight)) {
        throw new Error('Expected number for `scanHeight` argument, but got ' + typeof scanHeight);
    }

    if (_.isNumber(scanTimestamp)) {
        throw new Error('Expected number for `scanTimestamp` argument, but got ' + typeof scanTimestamp);
    }
    // ...
}

It might make sense to extract these into generic methods, something like assertIsNumber(scanHeight) and then use some wizardy to get the name of the passed variable.

Only needs to apply to user facing (exported) functions.

Might be a good idea to split this into multiple PR's - not sure how many functions will need changing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.