firebase / geofire-js

GeoFire for JavaScript - Realtime location queries with Firebase
MIT License
1.44k stars 345 forks source link

Puf strengthen types #215

Closed puf closed 3 years ago

puf commented 3 years ago

Description

This PR makes the GeoFire API more type-safe by introducing three new types:

This change does not make any change to the actual underlying code, and the signatures are backwards compatible to any correctly working code on the previous API. The only difference after this change is that more errors will be caught at compile time (when TypeScript is converted to JavaScript), rather than by the extensive runtime checks that GeoFire performs on them.

For googlers: the full list of type changes can be found on go/geofire-types.

coveralls commented 3 years ago

Coverage Status

Coverage increased (+0.3%) to 92.202% when pulling 49c1d04be9f714271339c33b92ea0539bd605544 on puf-strengthen-types into 8d13bcd3df606cebfb20168e448033d3cc98f8ae on master.

puf commented 3 years ago

Don't merge yet, as I think I still need to move some tests - which should also improve coverage again.

But aside from that, the main code could do with a thorough review.

puf commented 3 years ago

OK, I think that covers it for now. @samtstern : Can you review, mostly focusing on the signature changes of the API to ensure they're indeed non-breaking? See go/geofire-types 🔐 for reference of the changes.