Reconsider invariant-stype type guard

[rafaelweinstein]

I hate to pile onto this extremely lengthy discussion, but I feel like I need to.

I realize this is a duplicate of #19066 and apropos of #10421.

For context, I'm working at Salesforce Quip (https://github.com/quip) on what will likely end up being a near year-long effort to transform roughly 500K lines of code of globally namespaced, Google Closure type-annotated JS to TypeScript -- using a custom transformation pipeline.

We've already converted the global namespace to ES6 modules and we've gotten the TSC error rate down below 10k.

The lack of something like flow's invariant or GCC's debug.assert is a problem. We have O(1000s) of calls to debug.assert() and the existing code was written such that GCC uses it as a type-refinement condition.

The thing that something like invariant accomplishes that #10421 would not (if I'm misunderstanding, please tell me) is that invariant allows you to run code.

This is important, because we have a system of diagnostic reporting in which client side errors are captured and submitted to tracking infrastructure on the server. Simply throwing isn't really an option. It's also the case, that in production, we actively do not want to stop execution when the condition fails. In many cases, the failed assertion wouldn't have resulted in user-visible failures.

I could imagine using the never type as workaround here, but it doesn't seem to cause type refinement in the same way that throw Error() does. e.g.

function foo(a: string | number): number {
  if (typeof a === "string") {
    unreached(); // Replace me with "throw new Error();" and this compiles
  }
  return a;
}

function unreached(): never {
  throw new Error();
}

[MartinJohns]

Just write:

throw unreached();

And yes, it's a duplicate of #10421, which you are aware of already.

[rafaelweinstein]

@MartinJohns, I realize that throw unreached() or return unreached() will cause the type refinement, but it also stops execution along the existing code path and throws, neither of which we want (in particular in production).

That was the new (I think) part of this discussion which I was attempting to note.

[RyanCavanaugh]

I want to concretely convey that this is something we want to have, but have not yet found a palatable solution which delivers the intended effects without having unacceptable performance impact.

[rafaelweinstein]

I appreciate that, @RyanCavanaugh.

a) Do you have any suggestions for how to transform our existing debug.assert calls which results in type refinement such that we can control whether an exception is thrown at runtime?

b) I had a hard time wading through the lengthy existing discussions of this issue. Can you point me into the any relevant discussion outlining the design constraints that you are grappling with?

I'd be interested to know what you think the "right" solution is, absent performance considerations.

[RyanCavanaugh]

Re (A): Honestly the use case baffles me and I don't have any proposals. If the type guard fails, you're either going to crash shortly, or corrupt data in a very difficult-to-manage way. If you really want to proceed down that road, why not just write this? A type assertion is extremely correct in terms of expressing your intent to do something that is very possibly false.

function foo(a: string | number): number {
  if (typeof a === "string") {
    unreached(); // Replace me with "throw new Error();" and this compiles
  }
  return a as number;
}

Re (B): Probably best to start at #8655

The "right" solution if computers were infinitely fast is straightforward - put every function call in the control flow graph and add a type guard return type for "throws if T is (not?) U"

[RyanCavanaugh]

#31512 (comment)

[rafaelweinstein]

There's probably not much use to debating the legitimacy of the feature. I totally understand your view (and to be honest, it's more akin to my own sensibilities than not). That doesn't change the fact that we have a quite complex codebase and part of what makes a one-shot transform from GCC to TS possible is that the type systems (modulo enums) largely have no impact on runtime semantics.

With regard to feasibility, I'm way out of my depth here, but FWIW, it seems to me that Flow's original and still current (facebook/flow#112) solution is quite a good trade off. I have no idea how it's actually implemented, but my mental model is that it's a very effective hack.

Effectively anywhere

invariant(condition); is encountered, logically re-write the AST to be

if (!condition) { throw Error(); } before you do flow & type analysis.

This would also address the request in #31512, albeit in a pretty low-tech way.

I don't care to die on this hill.

TypeScript is insanely awesome (thus me investing the past year of my work life trying to move the Quip team to it). Thanks for all your hard work.

[typescript-bot]

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.