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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small cleanups: fix peer-dep warnings, yarn-deduplicate, use Flow's exact_by_default. #83

Merged
merged 9 commits into from May 27, 2021

Conversation

chrisbobbe
Copy link
Contributor

@chrisbobbe chrisbobbe commented May 25, 2021

Thanks for your important work on this!

My goals here are to make a few small project-level fixes, without changing user-facing behavior. Please don't hesitate to let me know if something isn't clear or seems wrong.

But in particular, after this series of commits:

  • The warning output from running yarn is now down to zero, so new warnings will be conspicuous (these can be important)
  • Several dependencies are deduplicated, and yarn-deduplicate is left in as a tool that people can use to clean up commits that change dependencies.
  • Flow now treats object types as exact by default, which will make it easier to get tighter type checking in lots of places

I've tested this by running yarn lint and yarn flow at each commit (running yarn before those commands, on the commits that change the dependencies), and I've also tested the tip commit by running yarn frontend鈥攖hough without also running yarn backend, since that seemed like a bit of work to get fully set up. I'd really appreciate if you'd run these commits (or perhaps just the tip commit) through your usual manual tests, and other automated tests I might have missed, or else give me a nudge to get yarn backend working and I'll give it a try. 馃檪

When I ran yarn frontend at the tip of this series of commits, I was able to see this (and I presume I'd see more if I got set up to run yarn backend too); the loading indicator goes forever because it can't load any data:

image

The particular error message I've encountered with yarn backend is

/bin/sh: pipenv: command not found
error Command failed with exit code 127.

and presumably it wouldn't be hard for me to install something and get past this error, I just haven't yet.

@chrisbobbe chrisbobbe changed the title Small cleanups: fix peer-dep warnings, yarn-deduplicate, Flow's exact_by_default. Small cleanups: fix peer-dep warnings, yarn-deduplicate, use Flow's exact_by_default. May 25, 2021
@kevinjcliao
Copy link
Collaborator

Hi Chris! To get the backend to work, you should install pipenv. I'm using it as a sort of NPM for Python (per-project dependency management).

https://pipenv.pypa.io/en/latest/

Installation instructions there^^

Let me know if you have any questions, but this looks fantastic. Given you haven't touched Python, the Python checks shouldn't be necessary. I'll see if the GitHub Actions pass.

@chrisbobbe
Copy link
Contributor Author

chrisbobbe commented May 26, 2021

Got it, yarn backend is working now!

@chrisbobbe
Copy link
Contributor Author

chrisbobbe commented May 26, 2021

OK, and after running yarn backend and yarn frontend, and visiting http://localhost:5000 that was printed from yarn backend, I've poked around and things seem to be working correctly. Being new to the project, I may be less likely to spot things going wrong, but nothing stood out to me. Most of the changes deal with types and dev dependencies.

I can't say it's impossible that the yarn-deduplicate commands disrupted something, but I think it's unlikely. yarn-deduplicate has proven to be reliable, at least in the versions of it that we've used in zulip-mobile. It's been running on all commits in https://github.com/zulip/zulip-mobile since zulip/zulip-mobile@8b155e9, in 2019-11. Keep in mind, though, that not all packages follow semver (and it can get pretty close to impossible to follow perfectly), so there's always some chance that a correct deduplication does something unexpected somewhere. That's happened once in the zulip-mobile project that came to my attention; context is at zulip/zulip-mobile#4152 (comment) and following, and a linked chat thread here.

I do think that the benefits of deduplicating outweigh the risks. ~All versions of a given package have some bugs, and it's easier to be acquainted with them if we don't have lots of instances of a package lying around at lots of different versions.


Observations from my manual testing:

  • I made a temporary, obvious change to have the whole page show "HELLO WORLD" just to make sure it wasn't serving something that might have been cached from before my changes; that checked out (page updated automatically with the new content)
  • The language picker did what I expect
  • The "About", "Appointments", "Vaccine Criteria", and "Credits" pages all showed nontrivial content
  • "Appointments" took several seconds to load, but then showed data (screenshots below). As expected, it looks like the tip commit, renaming VaccineInfo, didn't prevent that stuff from showing up.

image
image
image

Copy link
Collaborator

@kevinjcliao kevinjcliao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes to remove the ID, then happy to accept!

Comment on lines 36 to 40

// `Card` has a caller that passes this, but `Card` doesn't do
// anything with it. Should we remove it?
hospitalId: number, // eslint-disable-line react/no-unused-prop-types

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this was a mistake. Do you mind removing it?

Done with this command, using Yarn 1.22.10:

  yarn add --dev yarn-deduplicate

This will help us trim out some dependencies that were installed
multiple times, at different versions, when version-range
constraints would actually allow having one installation at a
particular version:
  https://www.npmjs.com/package/yarn-deduplicate
  https://github.com/atlassian/yarn-deduplicate
yarn-deduplicate, which we added to `devDependencies` in the
previous commit, cleans up `yarn.lock` by removing duplicates:
  https://www.npmjs.com/package/yarn-deduplicate
  https://github.com/atlassian/yarn-deduplicate

Done using Yarn 1.22.10. Note that the package's docs (linked just
above) report that the package is obsolete when using Yarn v2
because Yarn v2 supports package deduplication natively. Since this
command found something to do, I assume we haven't been using Yarn
v2. We could go to the trouble of upgrading, but it looks like
there's some nontrivial work involved, and some packages might not
yet work well with v2, and I have no experience with v2:
  https://yarnpkg.com/getting-started/qa#why-should-you-upgrade-to-yarn-modern
Done with the following commands, using Yarn 1.22.10:

  yarn add @types/react --latest
  yarn add @babel/core postcss --dev --latest
  yarn yarn-deduplicate && yarn

after seeing the warnings below, on an initial run of `yarn` (i.e.,
one that starts with `node_modules` not existing).

(The `yarn-deduplicate` command is a routine cleanup command we
added in a recent commit. It found something to do after running the
other two commands.)

Each line in the output below represents an assertion from one of
our project's dependencies that our project must add a direct
dependency on { @types/react, @babel/core, postcss } at a version in
a specific range:
  https://docs.npmjs.com/cli/v7/configuring-npm/package-json#peerdependencies
  https://nodejs.org/es/blog/npm/peer-dependencies/

Luckily, all the ranges for each of those three packages are wide
enough that they intersect at at least one specific version. Even
better: for all three packages, the intersection isn't just *one*
specific version -- it includes the default range that gets
specified when you just `yarn add ... --latest` the package. So, do
that, for all three.

From the output, it looks like the @types/react warning stems from
our top-level dependency react-markdown, and that's under
`dependencies`. So, install @types/react under `dependencies`.

From the output, it looks like all the @babel/core and postcss
warnings stem from parcel, which is under `devDependencies`. So,
install @babel/core and postcss under `devDependencies`.

warning " > react-markdown@6.0.1" has unmet peer dependency "@types/react@>=16".
warning "parcel > @parcel/config-default > @parcel/transformer-babel@2.0.0-beta.2" has unmet peer dependency "@babel/core@^7.12.0".
warning "parcel > @parcel/config-default > @parcel/transformer-postcss@2.0.0-beta.2" has unmet peer dependency "postcss@^8.2.1".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/helper-compilation-targets@7.13.16" has unmet peer dependency "@babel/core@^7.0.0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/plugin-transform-flow-strip-types@7.13.0" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/plugin-transform-typescript@7.13.0" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env@7.13.15" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-react@7.13.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @parcel/babel-preset-env@2.0.0-beta.2" has unmet peer dependency "@babel/core@^7.12.0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/plugin-transform-flow-strip-types > @babel/plugin-syntax-flow@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/plugin-transform-typescript > @babel/helper-create-class-features-plugin@7.13.11" has unmet peer dependency "@babel/core@^7.0.0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/plugin-transform-typescript > @babel/plugin-syntax-typescript@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-bugfix-v8-spread-parameters-in-optional-chaining@7.13.12" has unmet peer dependency "@babel/core@^7.13.0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-proposal-async-generator-functions@7.13.15" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-proposal-class-properties@7.13.0" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-proposal-dynamic-import@7.13.8" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-proposal-export-namespace-from@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-proposal-json-strings@7.13.8" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-proposal-logical-assignment-operators@7.13.8" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-proposal-nullish-coalescing-operator@7.13.8" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-proposal-numeric-separator@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-proposal-object-rest-spread@7.13.8" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-proposal-optional-catch-binding@7.13.8" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-proposal-optional-chaining@7.13.12" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-proposal-private-methods@7.13.0" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-proposal-unicode-property-regex@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-syntax-async-generators@7.8.4" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-syntax-class-properties@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-syntax-dynamic-import@7.8.3" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-syntax-export-namespace-from@7.8.3" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-syntax-json-strings@7.8.3" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-syntax-logical-assignment-operators@7.10.4" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-syntax-nullish-coalescing-operator@7.8.3" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-syntax-numeric-separator@7.10.4" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-syntax-object-rest-spread@7.8.3" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-syntax-optional-catch-binding@7.8.3" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-syntax-optional-chaining@7.8.3" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-syntax-top-level-await@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-arrow-functions@7.13.0" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-async-to-generator@7.13.0" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-block-scoped-functions@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-block-scoping@7.13.16" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-classes@7.13.0" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-computed-properties@7.13.0" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-destructuring@7.13.17" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-dotall-regex@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-duplicate-keys@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-exponentiation-operator@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-for-of@7.13.0" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-function-name@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-literals@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-member-expression-literals@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-modules-amd@7.13.0" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-modules-commonjs@7.13.8" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-modules-systemjs@7.13.8" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-modules-umd@7.13.0" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-named-capturing-groups-regex@7.12.13" has unmet peer dependency "@babel/core@^7.0.0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-new-target@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-object-super@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-parameters@7.13.0" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-property-literals@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-regenerator@7.13.15" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-reserved-words@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-shorthand-properties@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-spread@7.13.0" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-sticky-regex@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-template-literals@7.13.0" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-typeof-symbol@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-unicode-escapes@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-transform-unicode-regex@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/preset-modules@0.1.4" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > babel-plugin-polyfill-corejs2@0.2.0" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > babel-plugin-polyfill-corejs3@0.2.0" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > babel-plugin-polyfill-regenerator@0.2.0" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-react > @babel/plugin-transform-react-display-name@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-react > @babel/plugin-transform-react-jsx@7.13.12" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-react > @babel/plugin-transform-react-jsx-development@7.12.17" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-react > @babel/plugin-transform-react-pure-annotations@7.12.1" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > @babel/plugin-proposal-unicode-property-regex > @babel/helper-create-regexp-features-plugin@7.12.17" has unmet peer dependency "@babel/core@^7.0.0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-env > babel-plugin-polyfill-corejs2 > @babel/helper-define-polyfill-provider@0.2.0" has unmet peer dependency "@babel/core@^7.4.0-0".
warning "parcel > @parcel/config-default > @parcel/transformer-babel > @babel/preset-react > @babel/plugin-transform-react-jsx > @babel/plugin-syntax-jsx@7.12.13" has unmet peer dependency "@babel/core@^7.0.0-0".
`Number` is for the wrapper object, i.e., the result of calling
`new Number(...)`, so it's rarely quite the right thing to use. The
lowercase form is for literal values [1] . Unfortunately, code
editors will sometimes suggest the capitalized type in autocomplete,
which is unhelpful.

[1] https://flow.org/en/docs/types/primitives/
This small error would be caught when we make more of our object
types exact, in the next commit.

Passing the prop wasn't intentional [1], so stop passing it.

[1] g0v#83 (comment)
It looks like all these object types would benefit from being exact;
so, make them exact. These were using the implicitly inexact syntax
(`{foo: number}`), not the explicitly inexact syntax
(`{foo: number, ...}`), but they were still inexact.

We know we've caught all implicitly inexact object types because the
`implicit-inexact-object` [1] lint is set to `error`, and Flow
doesn't report any errors at this commit.

We're about to turn on `exact_by_default` [2], which will change the
meaning of the `{foo: number}` syntax to implicitly *exact*, leaving
the option of `{| foo: number |}` for explicitly exact, and
`{foo: number, ...}` for explicitly inexact.

That'll make it easier to get tighter type checking on object types
in general, since most of the time we want them to be exact.

[1] https://flow.org/en/docs/linting/rule-reference/#toc-implicit-inexact-object
[2] https://flow.org/en/docs/config/options/#toc-exact-by-default-boolean
This was a much-anticipated feature [1] that's available in recent
versions of Flow.

We'll still be able to make object types inexact, but we'll have to
do it explicitly, with a marked syntax. That seems appropriate:
since we don't want to use them as often, it makes sense for them to
be marked, and for exact object types to be unmarked.

As explained in the docs [2],

````
Set this to `true` to indicate that Flow should interpret
object types as exact by default. When this flag is `false`, Flow
has the following behavior:

```js
{foo: number} // inexact
{| foo: number |} // exact
{foo: number, ...} // inexact
```

When this flag is `true`, Flow has the following behavior:

```js
{foo: number} // exact
{| foo: number |} // exact
{foo: number, ...} // inexact
```

The default value is `false`.
````

[1] https://medium.com/flow-type/on-the-roadmap-exact-objects-by-default-16b72933c5cf
[2] https://flow.org/en/docs/config/options/#toc-exact-by-default-boolean
In the previous commit, we enabled the `exact_by_default`
option [1].

Now, we can make our exact object types unmarked. Since we expect to
use exact object types more often than inexact ones, it makes sense
for the exact ones to be unmarked, and to require the inexact ones
to be marked (with `, ...` as in `{foo: number, ...}`).

To confirm that this works, try adding a `nonsense="123"` prop to
the `Card` callsite in frontend/Components/VaccineInfo/Cards.jsx.

[1] https://flow.org/en/docs/config/options/#toc-exact-by-default-boolean
@chrisbobbe
Copy link
Contributor Author

Thanks for the review! Revision pushed.

Copy link
Collaborator

@kevinjcliao kevinjcliao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! Thank you for your help!

@kevinjcliao kevinjcliao merged commit 5f34b1c into g0v:master May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants