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

expectError strict behaviour #40

Open
SamVerschueren opened this issue Sep 30, 2019 · 4 comments
Open

expectError strict behaviour #40

SamVerschueren opened this issue Sep 30, 2019 · 4 comments
Labels
help wanted Extra attention is needed question Further information is requested

Comments

@SamVerschueren
Copy link
Collaborator

I need feedback on the behaviour of the expectError assertion.

The expectType assertion is strict. Which means that expectType<string | number>(concat('a', 'b')) will fail because string | number is defined too wide. See the docs.

I didn't change anything for expectError as I wasn't sure on the exact behaviour.

@cartant asked if it would behave like this (see ReactiveX/rxjs#4871 (comment)).

expectError<string | number>(add(1, 2));
//=> no error

The reason behind it could be that because the type is too wide, the assertion is an error and that's what you expect.

I also discussed this with @sindresorhus and he indicated that maybe we should hard fail on that assertion.

I definitely could use feedback on this topic here :).

// @BendingBender

@SamVerschueren SamVerschueren added help wanted Extra attention is needed question Further information is requested labels Sep 30, 2019
@cartant
Copy link

cartant commented Sep 30, 2019

What would be the reason for wanting expectError to effect an error if the expression's type is anything other than an exact match for the specified type argument?

IMO, if it were to behave in that way, it would be a little confusing, as there would be no TypeScript error effected when assigning to a wider type, so there is no error to expect. If it's desirable for it to behave like that, I think it needs a new name that better describes what it does. Something like expectNotType.

BTW, the expectations for testing deprecations - that I mentioned in that RxJS issue - would need a similar pair of functions: expectDeprecated and expectNotDeprecated. I mention this here because like expectType and (maybe) expectNotType they are duals. I will open another issue to discuss the deprecation expectations.

@cartant
Copy link

cartant commented Sep 30, 2019

Perhaps these situations would be simpler if the package include a general negation mechanism for expectations?

@cartant
Copy link

cartant commented Nov 5, 2019

Have you given any further consideration to this?

IMO, an expectation named expectError should only fail if assignment of the types under test would see the TypeScript compiler effect an error. The assignment of a narrower type to a wider type wouldn't effect an error, so the expectation should not effect a failure.

@cartant
Copy link

cartant commented Nov 5, 2019

Further to the above, with RxJS we use dtslint to test arguments that are passed to functions - presumably expectError can be used to to this?

With arguments, I would expect an expectation failure to be effected only if the TypeScript compiler effects an error. I would not expect a failure to be effected when passing an argument that has a type that's narrower than the parameter type. And, given that, I would expect the return value to be treated in a similar manner: no expectation failure unless the TypeScript compiler effects an error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants