Skip to content

Make it way more obvious that objects need unique identifiers#3

Open
keevie wants to merge 2 commits intomainfrom
handle-array-ids
Open

Make it way more obvious that objects need unique identifiers#3
keevie wants to merge 2 commits intomainfrom
handle-array-ids

Conversation

@keevie
Copy link
Contributor

@keevie keevie commented Aug 12, 2021

No description provided.

test/auto.js Outdated

let doc3 = AutoPigeon.applyChanges(doc1, c1);

assert(JSON.stringify(doc3.log) === JSON.stringify(changed));
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use assert.deepEqual here?

function _objId(x) {
if (_typeof(x) == 'object') {
const id = x.id || x._id;
const id = x.id || x._id || x.uuid || x._uuid;
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering if we can allow for for the user to specify/override their own function here...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really like that idea, but when I started to think about it it didn't seem at all obvious where to pass in that option -- the static from method seems not really right, and there doesn't seem to be another spot where we initialize.

Maybe that's something we should change though? It would be nice to be able to pass in options for history length too.

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried a take on this in custom-id-function branch

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.

2 participants