-
-
Notifications
You must be signed in to change notification settings - Fork 214
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
Convert to TS project #423
base: main
Are you sure you want to change the base?
Convert to TS project #423
Conversation
I started looking at this too. I used this script to do the bulk rename. You might find it useful. #!/usr/bin/env bash
set -o errexit -o pipefail -o nounset
function moveFiles() {
dir="${1}"
cd "${dir}"
echo "Renaming files in ${PWD}"
if compgen -G "*.js" > /dev/null; then
for file in *.js; do
git mv "${file}" "${file%.js}.ts"
done
fi
if compgen -G "*.js.snap" > /dev/null; then
for file in *.js.snap; do
git mv "${file}" "${file%.js.snap}.ts.snap"
done
fi
cd - > /dev/null
}
moveFiles .
moveFiles src
moveFiles src/all
moveFiles src/matchers
moveFiles src/utils
moveFiles test/matchers
moveFiles test/utils
moveFiles test/matchers/__snapshots__ |
Sorry for the late reply on this. Since, as you say, Jest's built-in matchers do runtime checks, so it seems reasonable to do it here as well. |
No problem. |
I needed an opinion before proceeding with the other files, I thought of doing the validations as follows. And I'll probably only add the validations to matchers that do something with received value x expected value made a type-guard function to narrow down type and throw a error // src/utils.ts
export function validateType<A>(condition: boolean, value: unknown, message: string): asserts value is A {
if (!condition) {
throw new Error(message);
}
}
// src/toBeAfter (for example)
``ts
export function toBeAfter(this: jest.MatcherContext, actual: unknown, expected: Date): jest.CustomMatcherResult {
const {
printReceived,
printExpected,
matcherHint,
matcherErrorMessage,
RECEIVED_COLOR,
EXPECTED_COLOR,
printWithType,
} = this.utils;
const matcherName = 'toBeAfter';
validateType<Date>(
actual instanceof Date,
actual,
matcherErrorMessage(
matcherHint(matcherName),
`${RECEIVED_COLOR('received')} value must be a Date.`,
printWithType('Received', actual, printReceived),
),
);
// not need pass type parameter because expected is typed.
validateType(
expected instanceof Date,
expected,
matcherErrorMessage(
matcherHint(matcherName),
`${EXPECTED_COLOR('expected')} value must be a Date.`,
printWithType('Expected', expected, printExpected),
),
);
const passMessage =
matcherHint(`.not.${matcherName}`, 'received', '') +
'\n\n' +
`Expected date to be after ${printReceived(expected)} but received:\n` +
` ${printReceived(actual)}`;
const failMessage =
matcherHint(`.${matcherName}`, 'received', '') +
'\n\n' +
`Expected date to be after ${printReceived(expected)} but received:\n` +
` ${printReceived(actual)}`;
const pass = actual > expected;
return { pass, message: () => (pass ? passMessage : failMessage) };
}
`` |
Regarding this function I didn't really like the fact that I'm passing the value but currently I'm not doing anything with him. // src/utils.ts
export function validateType<A>(condition: boolean, value: unknown, message: string): asserts value is A {
if (!condition) {
throw new Error(message);
}
} Maybe separate several validation functions for each type to remove the parameter value? export function validateString(value: unknown, message: string): asserts value is string {
if (typeof value !== 'string') {
throw new Error(message);
}
}
export function validateObject(value: unknown, message: string): asserts value is Record<string, unknown> {
if (typeof value !== 'object' && value === null) {
throw new Error(message);
}
} |
I think I'd suggest maybe holding off on the validation for now and picking that up in a separate PR. We're not currently checking the types (or at least we're not for all matchers), so it's not functionality we're losing. Plus, adding the validations (where they didn't exist before) is I think maybe a breaking change, so it'd probably be nice to release that separately too. |
I hope that with this pull-request I'm not getting in the way of anyone working on it
What
Why
I believe it will be easier to add new types by letting the typescript handle the part of creating the .d.ts files.
It will make it easier to maintain if jest happens to made a breaking change, we will be able to track this easier because of the types from jest.
Notes
Housekeeping