Commit Graph

8 Commits

Author SHA1 Message Date
Greg Price 0de1d03dd4 typing_status: Mark recipient array as allowing number subtypes.
Concretely, we'll use this with a `UserId` type which is an
"opaque type alias" of `number` -- it's secretly implemented as
simply `number`, and it can be consumed by anything that wants a
`number` (in other words, it's a subtype of `number`), but the
fact that it secretly just is `number` is private to the module
that defines the type.

As far as the typing_status code is concerned, allowing this to
be a subtype of `number` just means that the code doesn't ever
try to inject new numbers of its own into the recipients arrays
that it passes around.
2020-12-30 14:55:24 -08:00
Greg Price 9a15301f4a typing_status: Mark recipient array as read-only for Flow.
This type means that code consuming this value promises not to
mutate it.  It's useful partly for the sake of simply controlling
mutation, so that arrays can be passed around without making
defensive copies; and partly because it makes the type covariant
in the elements, rather than invariant.

That is, if a function takes a plain Array<number | null>, then you
can't pass it an Array<number>, because it might add a `null` to it.
But if it takes $ReadOnlyArray<number | null>, then you can.
In general, Array<S> <: $ReadOnlyArray<S> <: $ReadOnlyArray<T>
for any S <: T, where `<:` means "is a subtype of".

Marking this type as read-only means we can pass in a read-only
array without adding a fixme (equivalent to a mypy type-ignore) to
locally disable the type-checker, nor a redundant defensive copy.
2020-12-30 14:55:24 -08:00
Greg Price 71596648c2 typing_status: Switch sentinel "recipient" value to `null`.
This feels a bit more semantically appropriate: it more clearly says
"here's some information: there is no (relevant) recipient", rather
than "no information available".  (Both `null` and `undefined` in JS
can have either meaning, but `undefined` especially commonly means
the latter.)

Concretely, it ensures a bit more explicitness where the value
originates: a bare `return;` becomes `return null;`, reflecting the
fact that it is returning a quite informative value.

Also make the implementation more explicit about what's expected here,
replacing truthiness tests with `!== null`.  (A bit more idiomatic
would be `!= null`, which is equivalent when the value is well-typed
and a bit more robust to ill-typing bugs.  But lint complains about
that version.)
2019-10-24 14:56:56 -07:00
Greg Price a191890213 typing_status: Fold `stop` into main method `update`.
It'd already been the case for some while that calling `stop` had the
same effect as calling `update` (previously `handle_text_input`) with
a falsy recipient.  With the API changes in the previous few commits,
this becomes quite natural to make explicit in the API.
2019-10-24 14:56:56 -07:00
Greg Price e639b0a6f8 typing_status: Write jsdoc for main entry point, and rename.
This was named after when it gets called from the UI, rather than
after what it can be expected to do.

Naming it after what it's meant to do -- and giving a summary line to
expand on that -- provides a more helpful semantic idea for reasoning
about the function.  Doubly so for using the function in a different
client with its own UI, like the mobile app.
2019-10-24 14:56:56 -07:00
Greg Price dcb5bb7914 typing_status: Combine two parameters into one, with a maybe-type.
The main motivation for this change is to simplify this interface
and make it easier to reason about.

The case where it affects the behavior is when
is_valid_conversation() returns false, while current_recipient
and get_recipient() agree on some truthy value.

This means the message-content textarea is empty -- in fact the
user just cleared it, because we got here from an input event on
it -- but the compose box is still open to some PM thread that we
have a typing notification still outstanding for.

The old behavior is that in this situation we would ignore the
fact that the content was empty, and go ahead and prolong the
typing notification, by updating our timer and possibly sending a
"still typing" notice.

This contrasts with the behavior (both old and new) in the case
where the content is empty and we *don't* already have an
outstanding typing notification, or we have one to some other
thread.  In that case, we cancel any existing notification and
don't start a new one, exactly as if `stop` were called
(e.g. because the user closed the compose box.)

The new behavior is that we always treat clearing the input as
"stopped typing": not only in those cases where we already did,
but also in the case where we still have the same recipients.
(Which seems like probably the common case.)

That seems like the preferable behavior; indeed it's hard to see
the point of the "compose_empty" logic if restricted to the other
cases.  It also makes the interface simpler.

Those two properties don't seem like a coincidence, either: the
complicated interface made it difficult to unpack exactly what
logic we actually had, which made it easy for surprising wrinkles
to hang out indefinitely.
2019-10-24 14:56:56 -07:00
Greg Price 5c220ed11a typing_status: Use parameters for data rather than callbacks.
The real purpose these two callbacks serve is exactly what an ordinary
parameter is perfect for:
 * Each has just one call site, at the top of the function.
 * They're not done for side effects; the point is what they return.
 * The function doesn't pass them any arguments of its own, or
   otherwise express any internal knowledge that doesn't just as
   properly belong to its caller.

So, push the calls to these callbacks up into the function's caller,
and pass in the data they return instead.

This greatly simplifies the interface of `handle_text_input` and of
`typing_status` in general.
2019-10-24 14:56:56 -07:00
Greg Price b70b7df22c shared: Describe interface of typing_status in Flow.
This allows the mobile app to stay well-typed while using this code.
2019-10-17 16:48:23 -07:00