-
Notifications
You must be signed in to change notification settings - Fork 12
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(firestore-sheets-sync): initial commit #52
base: main
Are you sure you want to change the base?
Conversation
… into @invertase/firestore-google-sheets
@@ -0,0 +1,119 @@ | |||
name: sync-firestore-sheets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name: sync-firestore-sheets | |
name: sync-firestore-sheets |
Would suggest changing the name to match the convention of the other extensions, e.g. the service name goes first, so firestore-sheets-sync
(like the folder name)
- param: FIELDS_TO_SYNC | ||
label: Fields to sync | ||
description: > | ||
A list of comma-separated fields to sync to the Google Sheet. If left blank, no fields will be synced. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it not make more sense to Sync all fields if left empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Required since Firestore schemaless, making the user input these fields ensures only those fields are synced, since otherwise multiple records with different fields may not work since we only have a single header row in the sheet for all data rows
At least that's my understanding :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As Mike said, this is the reason the field names are required
const set1 = new Set(arr1); | ||
const set2 = new Set(arr2); | ||
return ( | ||
arr1.every(item => set2.has(item)) && arr2.every(item => set1.has(item)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could probably be more efficient;
- Check the
.size
is equal to each other - If it is, just compare a with b.
Co-authored-by: Mike Diarmid <[email protected]>
Co-authored-by: Mike Diarmid <[email protected]>
Co-authored-by: Mike Diarmid <[email protected]>
Co-authored-by: Mike Diarmid <[email protected]>
Co-authored-by: Mike Diarmid <[email protected]>
Co-authored-by: Mike Diarmid <[email protected]>
No description provided.