Convert all object types to exact, or explicitly-inexact #3452
Comments
gnprice
added
a-tools
blocked on other work
Similar to 7037393 a few months ago: done for the same reasons, and in the exact same way. The remaining cases that cause errors are mostly examples of #3451, so we can convert them to exact object types as we fix that. And then there are a handful of types that are genuinely meant to be inexact object types. When we upgrade soon to Flow v0.84 (as part of #3399), we can make those explicit; tracking that as #3452.
gnprice
removed
the
blocked on other work
|
#3399 is now done. |
|
Flow upstream recently posted an update on how to do this upgrade: Looking through the steps there:
We already have a few explicit inexact object types and haven't seen them break anything, so this is mostly set. Checking the four package versions mentioned in the post, we're good on three of them, but we should
The flowlint rule Looks like RN v0.60 brings us Flow v0.98, and RN v0.61 brings Flow v0.105. So that Flow upgrade, in turn, will come with the RN v0.61 upgrade #3781. OTOH that doesn't mean we need to wait to upgrade our code! There's a tool called
Not going to worry about this until step 2 is done, or at least mostly done. |
Does most of zulip#3452. TODO: * Apply Prettier. * Actually read the diff, and review. It's likely that a lot of these should be *exact*. Done with the following commands: $ yarn global add flow-upgrade $ flow-upgrade v0.83.0 v0.92.0
I just tried babel-eslint at v11.0.0-beta.2, and it wants at least ESLint 6. We'll get that in, e.g., #4128.
Now done, in #4151, great! |
|
Now that we're on RN v0.61 and Flow v0.105, we have that It finds 411 places. The majority of those are in the libdefs in Skimming through, there's a mix. Some are indeed meant to be inexact. A lot of them -- I think probably the majority -- are meant to be exact, and we just haven't gotten around to making them say so. So we don't want to just sweep through and automatically add This flowlint sure makes easy to find places that need this conversion! So the steps to push this forward now look like:
|
The default for a Flow object type is to be "inexact", but most types should actually be "exact". (The Flow maintainers agree, and have embarked on a migration to switch the default to exact; see our zulip#3452.) This one is typical in that respect.
As noted in a recent commit where we upgraded to Flow v0.125, we don't usually bump the Flow version outside of a React Native upgrade commit. But I found that we could get v0.126 without any added Flow errors in React Native code -- which is even better than the situation with Flow v0.125, where we saw one Flow error in one file. Being on v0.126 is nice because we'll finally be able to drop our comments like "this should really be exact -- see note in jsonable.js" on many objects with indexer properties, and make those object types exact. - That, in turn, will help us address zulip#3452 more productively ("Convert all object types to exact, or explicitly-inexact"). - Fixing zulip#3452 is really a prerequisite for taking facebook/react-native@050a7dd01 (switching on `exact_by_default`), which is on the path to the RN v0.64 upgrade. [1] https://flow.org/en/docs/config/options/#toc-exact-by-default-boolean
As noted in a recent commit where we upgraded to Flow v0.125, we don't usually bump the Flow version outside of a React Native upgrade commit. But I found that we could get v0.126 without any added Flow errors in React Native code -- which is even better than the situation with Flow v0.125, where we saw one Flow error in one file. Being on v0.126 is nice because we'll finally be able to drop our comments like "this should really be exact -- see note in jsonable.js" on many objects with indexer properties, and make those object types exact. - That, in turn, will help us address zulip#3452 more productively ("Convert all object types to exact, or explicitly-inexact"). - Fixing zulip#3452 is really a prerequisite for taking facebook/react-native@050a7dd01 (switching on `exact_by_default`), which is on the path to the RN v0.64 upgrade. [1] https://flow.org/en/docs/config/options/#toc-exact-by-default-boolean
…on`. The doc, at https://zulip.com/api/get-events, suggests that these types already include all fields that will be present. So, make them exact. An instance of zulip#3452.
…e it. An instance of zulip#3452.
An instance of zulip#3452.
An instance of zulip#3452.
An instance of zulip#3452.
An instance of zulip#3452.
An instance of zulip#3452.
An instance of zulip#3452. PR zulip#4465 is currently open for type-checking `MessageList`'s props.
…act. An instance of zulip#3452.
…xact. An instance of zulip#3452.
An instance of zulip#3452.
I'm not so sure we need a libdef for `prettier`, actually. Anyway, an instance of zulip#3452.
An instance of zulip#3452.
…exact. An instance of zulip#3452.
An instance of zulip#3452.
…iends. In particular, more like `WebViewError, `WebViewMessage`, etc., just above it. An instance of zulip#3452.
…act. An instance of zulip#3452.
…xact. An instance of zulip#3452.
An instance of zulip#3452.
I'm not so sure we need a libdef for `prettier`, actually. Anyway, an instance of zulip#3452.
An instance of zulip#3452.
…exact. An instance of zulip#3452.
(This becomes possible only with Flow v0.84, which we'll get as part of #3399; so it's blocked on that. And completely finishing it is blocked on later work in upstream Flow.)Generally almost all our object types should be exact object types. See discussion in 61d2e34 and 7037393... and see the many examples in
git log --grep exactof type errors that had been concealed by inexact object types and were discovered by making the object types exact.The Flow team agrees, and they have been making changes to make it easier to work that way across a codebase:
https://medium.com/flow-type/on-the-roadmap-exact-objects-by-default-16b72933c5cf
We've already gone and made the bulk of our object types exact; see 61d2e34, 7037393, and 8370002 . With Flow v0.84, it becomes possible to explicitly mark as inexact the few types we actually want to describe that way. This task is to
The text was updated successfully, but these errors were encountered: