Repository navigation
add a flag to disable () => void being subtype of () => a #8584
Description
Activity
Would we also ban function calls in expression statements unless those functions are
void?e.g. I imagine you have this problem
function doSomething(): Promise<number>; // BUG doSomething();
Reacted by Brian Kimyes, you are right, expression statements other than
voidare a major painwish there was a flag that would require the
voidoperator with themunhandled
Promise<r>(a result now or later) is a classic example, but it is also a misuse to ignoreOptional<r>(a result or nothing) orTried <r, e>(a result or an exception)all in all TypeScript is clearly favoring OOP over FP being yet another mainstream languages with a low entry level
There are a lot of functions with infrequently-useful return values. They can't be declared as returning
void, since you sometimes want to use the return, but it would be quite burdensome to have to use thevoidoperator whenever you don't.Node.prototype.appendChildcomes to mind.Whether a return value is "ignorable" or not isn't just determined by its type, either. People ignore the number
setTimeoutreturns (a timer ID) all the time, but ignoring the numberMath.floorreturns, for example, can only be a bug:var x = 3.14; Math.floor(x); console.log(x); // "Why isn't it 3?" ;)
So to properly detect bugs involving ignoring return values, the type system would have to distinguish between ignorable and unignorable return values, perhaps by intersection with
void:declare function setTimeout(...): number & void; // ignorable declare function floor(...): number; // not ignorable
It's an interesting idea, but one the TypeScript people would probably reject.
Reacted by Sean Vieira, Duan Yao, NN, Agostino Carandente, Suhair Zain, Arseniy Terekhin and Ahmed GubaraPeople ignore the number setTimeout returns (a timer ID) this
if you are (like us) working on a serious UI heavy application with a possibility to cancel any long running opertaion you must never ignore it
for a jquery powered home page, sure go ahead
believe me the hassle of being exact about ignoring the return value is nothing compared to the benefits of being able to catch more bugs at compile time, if you value the quality and your time
this all comes down to an old idea of making typescript stricter to ones who needs it (minority): #274
is this the same as #8240?
it's about rephrasing the last line in 3.11.3 Subtypes and Supertypes
Instad:
the result type of M is Void, or the result type of N is a subtype of that of M.
Should be:
the result type of N is a subtype of that of M.
I will bring it up for discussion. though I do nothing think we should be adding flags to change the behavior of the type system, unless it is a transitional phase (e.g. noImplictAny, strictNullChecks, etc..).
Some points from discussion
- No new flags unless absolutely unavoidable
- This is a very useful modifier for some functions
- e.g.
Array#reversereturn value is always safe to ignore, but ignoringArray#concat's return value is 99.9% a bug (thank Array designers for confusing API design!) - Too cumbersome for the vast majority of people to enforce
void func();on all non-voidfuncs
- e.g.
- We could do this if we had function attributes, but we don't
- Reconsider if we get attributes or other metadata system that would enable this
Reacted by Sean Vieira, David Rodrigue, Stanislav Panferov, ZpdDG4gta, Kevin Ryan, Cosmin Ababei, Agostino Carandente, Samuel Bodin, Suhair Zain and GjumAs I already said in #8240, this issue is must have to simplify work with all immutable data structures and immutable operations (
concatis a good example of immutable operation). People quite often forget that they need to writemodel = model.set('prop', 1)instead of just
model.set('prop', 1)And it's pretty hard to find the error in the large codebase. And it's also very annoying.
9 remaining items
There should be a new strict compiler option which does either of these:
- Either treat return value of
() => voidasunknown, or - Make assigning
() => not voidto() => voidan error
Reacted by Brian Kim- Either treat return value of
I came here from another issue which was discussing an option to mark a function such that it's return type cannot be unused. I'm unable to understand whether this issue is actually about it, so please forgive me if it isn't.
A useful scenario for the above mentioned feature is while using redux-thunk. We can create a thunk like subscribeToPlan(), but we might forget to actually dispatch it, leading to an error. This is usually not a problem when we create new thunks because we'll find that the new feature is not working as expected, but if we're migrating from a library which uses another convention to something like a thunk, we might miss a place if there are a lot of changes. If there was a way to mark a thunk such that it has to be used, we'd be able to catch them at compile time.
Reacted by GjumWe've been spending hours dealing with bugs that would been caught with this check. as Simon Buchan (@simonbuchan) had pointed out, this language feature is available in a lot of languages, and some even have it turned on by default. e.g. Swift
Would really like to see TypeScript adding support to this as well.
Also, if anyone knows if there is any workaround currently?
Either being eslint, tslint or anything that can help mitigate this?
Reacted by Andrii Fidria, andrei, Carl Patenaude-Poulin, Suhair Zain, satoren, Gjum and Juan CampaReacted by andrei and Juan CampaF# has this turned on by default for all functions as well.
Instead of global flag could this be a new utility type than can be applied where we want?
At least it would not be breaking changes but still could help solve this issue in the long term
Like the opposite of readonly #24509
If specified return or param should be checkedfunction compute(myNumber: readonly number): SideEffect<number>; function mutate(myNumber: SideEffect<number>): void;
With
readonlycombined withSideEffectyou would be able to know that a value has changed or not,
and if it could be a mistake.Invalid
var x = 3.14; Math.floor(readonly x); // WARN: "math.floor() do not modify the input but returns it, did you mean to use the result?"
var x = {foo: 'bar'}; x = modify(x: SideEffect<object>): any; // WARN: "compute() modifies the input but you reassigned the results to the original value, did you mean to check the input?"
var x = {foo: 'bar'}; modify(x: SideEffect<object>): any; // WARN: "compute() modifies the input but you never reuse it, did you mean to pass it by reference?"
Valid
var x = 3.14; x = Math.floor(readonly x);
var x: SideEffect<object> = {foo: 'bar'}; modify(x: SideEffect<object>): any;
Reacted by GjumI was directed here from #8240 ("Result value must be used") and I'm not sure why that one was closed and locked in favor of this issue, since both seem to be about different concepts.
This issue (coming from #8581) started out with a global compiler flag to disable assigning a non-void return type to a void return type, which (as RyanCavanaugh and mhegazy already pointed out in #8581/#8240) runs against the established understanding of assignability - it is indeed perfectly valid to ignore the result of many function calls, especially most existing JavaScript APIs.
#8240 is about marking individual functions, for example like bodinsamuel suggested, which presumably wouldn't require "function attributes" so it would be worth reconsidering.
That feature would be much less intrusive than a global change of compiler behavior.
It also fulfills these requirements:- It wouldn't be a breaking change in existing TypeScript / JavaScript code
- It wouldn't change the runtime behavior of existing JavaScript code
- It could be implemented without emitting different JS based on the types of the expressions
- It isn't a runtime feature (e.g. new expression-level syntax)
Please clarify whether this issue is about the global flag or per-function flag, and if the former, please reopen #8240 as it is a different and reasonable feature, while this issue is clearly controversial due to its intrusive nature.
Considering this again, I think the solution to OP is straightforward:
() => undefinedIf you want a function that truly returns nothing, that type is
() => undefinedif you want a function whose return type you pledge to ignore, that type is
() => voidFor "result must be used", I guess we need a new issue; these two have become too interlinked. Feel free to open.
Reacted by Gjum, AbdulKareem Nalband, Samuel Bodin and Harper AndrewsActually, on second thought, "result must be used" is exactly the same as "pure" (or has such substantial overlap that it's not really needed to have both); #7770
Ryan Cavanaugh (@RyanCavanaugh) I'm not sure I quite understand: are you suggesting that if the original code were:
declare function useCallback(f: () => void); declare function callback() => a; useCallback(callback); // no error, a is ignored by useCallback
that it is changed to:
declare function useCallback(f: () => undefined); declare function callback() => a; useCallback(callback); // error
here?
The problem here is that it's changing
useCallbackto fixcallbackcaring aboutabeing used, because:sometimes assuming that the result of a function might be ignored isn't safe
It's not really the best description for this issue, but I can't really agree with merging with pure either.
Here are some DOM examples of very-non-pure, but very-must-use functions:
// different results each time, but no use except for result: Date.now(); crypto.randomBytes(); // even after unwrapping promise, could return failure response fetch(); // locks the stream as a side-effect, but also useless without using the reader stream.getReader();
And in practice the changes for pure would be largely about checking pure function bodies, while must-use is about checking calls to must-use results, at least from it's approach in other languages.
If you like I could put this threads must-use specific detail into a fresh issue?
Reacted by Gjum and Ryan CavanaughRyan Cavanaugh (@RyanCavanaugh) can you reopen either #8240 or #29173 for return value must be used feature
Or either of those, I suppose!
Reopened #8240
Ryan Cavanaugh (@RyanCavanaugh) (psst, you actually only unlocked it, you didn't actually reopen it)
Reacted by Ryan CavanaughOops, thanks!
sometimes assuming that the result of a function might be ignored isn't safe, consider adding a flag that would disable such assumptions
followup of #8581