Improve error for incompatible card data type in createToken
#19
Conversation
Switch order of `createToken` overload. Add tests for expected typescript errors.
hofman-stripe
force-pushed
the
hofman/improve-create-token-type-error
branch
from
72ac38f to
a98b40c
Compare
There was a problem hiding this comment.
Neat!
I have some slight FUD (not blocking) about putting this in out test suite, since we tend to be extremely hesitant in deleting tests or changing assertions. If we upgrade TS and the error messages change to something worse, are we OK updating these tests? If we make some unrelated improvement to the types and the error messages get worse, are we OK updating the tests? It seems like this optimized a local minima in the DX (one out of many possible errors), but are pinning ourselves to that optimization in the test suite.
tests/types/errors.test.ts
Outdated
| ])('%s', (fileName, expectedError) => { | ||
| expect( | ||
| getError(path.resolve(__dirname, `./fixtures/${fileName}.ts`)) | ||
| ).toMatch(expectedError); |
There was a problem hiding this comment.
(2) Could also be a nice fit for snapshots, perhaps with a custom serializer.
There was a problem hiding this comment.
I think you're right above, we should relax the match, not make it more strict. The flattened error has a lot of "stack" which may vary over versions.
|
Yeah I was considering relaxing the tests:
That way the only thing we'd test is that the error for card token mentions the extra property, which is the best approximation we can get for a meaningful message. WDYT? |
|
Nice, relaxing the tests should help a lot. |
Switch order of the
createTokenoverload to give precedence to CardElement over IbanElement.Summary & motivation
See stripe/react-stripe-js#47
Testing & documentation
Added tests for expected typescript errors.