firebase / geofire-js

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

Add non-minified geofire-utils.js build target #197

Closed puf closed 3 years ago

puf commented 3 years ago

Description

This branch adds a new build target to distribute a separate version of geofire-utils.js, which can then be used without any dependency on the Realtime Database.

Code sample

import { encodeGeohash, geohashQueries, distance } from './geofire-utils.js';
samtstern commented 3 years ago

@puf I fixed the tests here: https://github.com/firebase/geofire-js/pull/198

You should merge master into your branch.

coveralls commented 3 years ago

Coverage Status

Coverage decreased (-1.9%) to 92.308% when pulling 74dce6c058969d462513d9b8984ff3118c8a8fd1 on puf-geofire-utils into e45e7664efc75c8a5e3d2718e57269397cc40fc9 on master.

samtstern commented 3 years ago

@puf I ran git clean -f -d -X to get rid of all generated files and then ran npm install && npm run bootstrap && npm run build and then I have the following files:

geofire

$ ls -Ral packages/geofire/dist
total 0
drwxr-xr-x   3 samstern  primarygroup    96B Sep 28 10:55 ./
drwxr-xr-x  12 samstern  primarygroup   384B Sep 28 10:55 ../
drwxr-xr-x  12 samstern  primarygroup   384B Sep 28 10:55 geofire/

packages/geofire/dist/geofire:
total 392
drwxr-xr-x  12 samstern  primarygroup   384B Sep 28 10:55 ./
drwxr-xr-x   3 samstern  primarygroup    96B Sep 28 10:55 ../
-rw-r--r--   1 samstern  primarygroup   559B Sep 28 10:56 GeoCallbackRegistration.d.ts
-rw-r--r--   1 samstern  primarygroup   3.4K Sep 28 10:56 GeoFire.d.ts
-rw-r--r--   1 samstern  primarygroup   557B Sep 28 10:56 GeoFireTypes.d.ts
-rw-r--r--   1 samstern  primarygroup   8.0K Sep 28 10:56 GeoQuery.d.ts
-rw-r--r--   1 samstern  primarygroup   973B Sep 28 10:56 databaseUtils.d.ts
-rw-r--r--   1 samstern  primarygroup    12K Sep 28 10:56 geofire.min.js
-rw-r--r--   1 samstern  primarygroup    48K Sep 28 10:56 index.cjs.js
-rw-r--r--   1 samstern  primarygroup   181B Sep 28 10:56 index.d.ts
-rw-r--r--   1 samstern  primarygroup    48K Sep 28 10:56 index.esm.js
-rw-r--r--   1 samstern  primarygroup    50K Sep 28 10:56 index.js

geofire-common

$ ls -Ral packages/geofire-common/dist
total 0
drwxr-xr-x   3 samstern  primarygroup    96B Sep 28 10:55 ./
drwxr-xr-x  12 samstern  primarygroup   384B Sep 28 10:55 ../
drwxr-xr-x   7 samstern  primarygroup   224B Sep 28 10:55 geofire/

packages/geofire-common/dist/geofire:
total 120
drwxr-xr-x  7 samstern  primarygroup   224B Sep 28 10:55 ./
drwxr-xr-x  3 samstern  primarygroup    96B Sep 28 10:55 ../
-rw-r--r--  1 samstern  primarygroup   4.0K Sep 28 10:55 geofire.min.js
-rw-r--r--  1 samstern  primarygroup    15K Sep 28 10:55 index.cjs.js
-rw-r--r--  1 samstern  primarygroup   5.3K Sep 28 10:55 index.d.ts
-rw-r--r--  1 samstern  primarygroup    14K Sep 28 10:55 index.esm.js
-rw-r--r--  1 samstern  primarygroup    16K Sep 28 10:55 index.js

I can clearly see the utilities you implemented in packages/geofire-common/dist/geofire/index.js ... is this not working for you?

That said I do think the rollup config in common needs some cleanup, right now it's 100% cribbed from geofire but it needs to be slightly tweaked.

puf commented 3 years ago

@puf I ran git clean -f -d -X to get rid of all generated files and then ran npm install && npm run bootstrap && npm run build and then I have the following files: ...

I end up with the same. Not sure what was different about it last week, but the cleanup indeed might have helped.

Should the packages/geofire-common/dist/geofire actually be packages/geofire-common/dist/geofire-common so that we can push it to npm as its own package?

puf commented 3 years ago

LGTM

We can improve coverage later, and will add tasks for strengthening the types.