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

Remove special case for invariant #112

Open
petehunt opened this issue Nov 24, 2014 · 11 comments
Open

Remove special case for invariant #112

petehunt opened this issue Nov 24, 2014 · 11 comments

Comments

@petehunt
Copy link

I'm finding it difficult to add type annotations to my function named invariant() because of this little guy:

let msg = "unsupported arguments in call to invariant()" in

It seems that many of these functions are special cased. I'm curious as to why, because I can't use any functions that share these names anymore.

@avikchaudhuri
Copy link
Contributor

Eventually the type system will be powerful enough to express the "signatures" of these functions, at which point their names will not need to be special-cased any more (since they can simply be declared to satisfy the necessary signatures).

In the short-term, we'll try to scope these names somewhat, so that they don't interfere with other things with the same names.

@samwgoldman
Copy link
Member

The proposal for refinement carrying boolean types in #34 isn't powerful enough, AFAICT, to express the type of invariant. Since TypeScript has already implemented that, it's probably a good time to start thinking about how we'd express something like invariant in the type language.

@leebyron
Copy link
Contributor

Here's a rough working but not long-term stable proposal:

invariant(predicate: any): $Asserts<predicate is true>

This uses the Ident "is" Type grammar proposed in #34. It uses @mroch's recent addition of boolean literals as Types. And it uses Flow's "this is an uncommon and undocumented" pattern of $Thing for communicating specific typing behavior.

@samwgoldman
Copy link
Member

I like this syntax and I think the syntax does communicate all the necessary information. I feel like the implementation might get tricky around the $Assert type, which should result in an abnormal event. I haven't looked into this enough yet to say whether it's realistic or not (maybe @avikchaudhuri can weigh in), but I'll give this some more thought.

mroch added a commit that referenced this issue Jan 5, 2016
Summary:
flow has special-cased support for some deprecated Facebook-isms like `copyProperties` and `mergeInto` (both replaced by `Object.assign`). since these hacks were implemented in `type_inference_js`, they weren't sensitive to context (e.g. any function named `merge` was hijacked) and applied to all Flow projects, not just Facebook.

This diff creates a `CustomFunT` type along with several magic type annotations (`$Facebookism$Merge`, for example), and uses those as sigils that are returned by the normal identifier lookup routine. So, if you write `const merge = require('merge'); merge(...)`, we'll treat it like a normal lookup to resolve `merge`, and then a normal `CallT`... except that the thing that gets returned is a `CustomFunT(..., Merge)`, which we handle specially.

In other words, in order for any of these magic functions to do anything anymore, they need to be defined in your project's libs. See the `facebookisms` test for an example.

Fixes #1206
Related to #112 (would fix it, except doesn't handle `invariant`)

Reviewed By: jeffmo

Differential Revision: D2782891

fb-gh-sync-id: b1d03eb9cf2cd44f9e5f99167a5e2d6725151917
@mroch mroch changed the title Special cases for static_upstream/core Support predicate types Jan 6, 2016
@mroch
Copy link
Contributor

mroch commented Jan 6, 2016

3e35bd4 should get the rest of the static_upstream/core special cases out of the way, except for invariant. Renamed this task to preserve the discussion of predicate types.

@mroch mroch changed the title Support predicate types Remove special case for invariant Jan 7, 2016
@gajus
Copy link

gajus commented May 22, 2018

invariant(predicate: any): $Asserts<predicate is true>

Was this ever implemented?

@leebyron
Copy link
Contributor

No, %checks was built which is for predicate functions but not assertion functions. If this is a model the flow team is happy with then maybe something like this could be built such as %asserts?

@gajus
Copy link

gajus commented May 23, 2018

For others reference, here is the documentation for %checks https://flow.org/en/docs/types/functions/#toc-predicate-functions.

@gajus
Copy link

gajus commented Jul 14, 2018

This issue is duplicate of #34.

@mattconde
Copy link
Contributor

So I think I've caught up on this issue, am I right to think I still can't flowtype my own "invariant" module correctly because of this?

@goodmind
Copy link
Contributor

goodmind commented Aug 4, 2019

TypeScript now has this microsoft/TypeScript#32695

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants