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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

duplicate id warning from observeSequence #1980

Closed
timtch opened this issue Mar 29, 2014 · 20 comments
Closed

duplicate id warning from observeSequence #1980

timtch opened this issue Mar 29, 2014 · 20 comments
Labels

Comments

@ghost
Copy link

@ghost ghost commented Mar 29, 2014

_1 Upvote_ Warnings in a browser console with array:
duplicate id -#ffffff in ["#3c78aa", "#ffffff", "#ffffff", "#ffffff", "#ffffff"] observe_sequence.js:6

    var example = Pages.findOne().themes 

    JSON.stringify(example) 
      [ 
          { "colors": 
              [ "#3c78aa", "#ffffff", "#ffffff", "#ffffff", "#ffffff" ] 
          } 
      ]
@aaronjudd
Copy link

@aaronjudd aaronjudd commented Mar 29, 2014

I'm also getting this with an json array that has duplicate values. Not sure I understand the reason for having an error here.

@Slava
Copy link
Member

@Slava Slava commented Mar 29, 2014

If you used this array ["#3c78aa", "#ffffff", "#ffffff", "#ffffff", "#ffffff"] as your return value for #each block helper, then this is what happened most likely:

Blaze took your array and rendered it. Since it needs to get updates when the array changes, it will try to observe it as it was a Meteor.Collection.Cursor.

Since it is not a cursor, Blaze will try to run diffing every time it gets two different arrays. In a process of diffing, diffing algorithm needs some kinds of id's to distinguish the values. Usually it would look at _id field but as you have plain strings in your array, it would treat these strings as id's.

But id's should be unique in order for diffing algorithm to be able to distinguish them, looking at the array of similar strings, diffing algorithm will report that ids are not actually unique.

@glasser
Copy link
Member

@glasser glasser commented Mar 29, 2014

Yeah maybe we should fall back to "just rerender whole sequence without
diffing" in some cases, like dupe ids.

@ghost
Copy link
Author

@ghost ghost commented Mar 29, 2014

can i use it as it is or should i need to add something like this [ { _id: "1234", color: "#3c78aa" }...] or some other way?

@glasser
Copy link
Member

@glasser glasser commented Mar 29, 2014

As a workaround you can add ids which should be distinct.

@aaronjudd
Copy link

@aaronjudd aaronjudd commented Mar 29, 2014

I clone documents into a new collection subdocument, where I keep their _id intact, and there can be multiple entries of the cloned documents. I'd prefer to not transform the _id into something new, as then I'd have extra work to reference the original - not hard, but just more work that I didn't have to do before. my 2 cents.

@Slava
Copy link
Member

@Slava Slava commented Mar 29, 2014

Does having duplicate ids break your app? Or just outputs warnings?

@aaronjudd
Copy link

@aaronjudd aaronjudd commented Mar 29, 2014

just outputs warnings, everything else still works correctly.

@ghost
Copy link
Author

@ghost ghost commented Mar 29, 2014

@glasser the idea with rerender the whole sequence is very cool :)

@avital
Copy link
Contributor

@avital avital commented Mar 29, 2014

To be clear, your app will work perfectly fine -- we dedup in the
observe-sequence. The only thing is that you might not get the semantics
you hope for when items move (elements can get re-rendered instead of just
moving). We can consider getting rid of the warning.

On Sat, Mar 29, 2014 at 1:24 PM, timtch notifications@github.com wrote:

@glasser https://github.com/glasser the idea with rerender the whole
sequence is very cool :)

Reply to this email directly or view it on GitHubhttps://github.com//issues/1980#issuecomment-39007799
.

@ghost
Copy link
Author

@ghost ghost commented Mar 29, 2014

Got it. Thanks guys!

@aaronjudd
Copy link

@aaronjudd aaronjudd commented Mar 29, 2014

I also considered that would be the side effect, and am weighing the pros/cons of doing a transform of the _id - if the warning were being generated only on _id I might do this, because right now I am doing some funky workarounds for the movement.. wasn't really sure which path was going to be more work. I'd probably rewrite if the warnings were only tied to _id, but I have other duplicate data - I'm assuming that in the case of @timtch if an unique _id is added even when the rest of the document is duplicated you no longer show a warning?

@avital avital changed the title duplicate id error from observeSequence duplicate id warning from observeSequence Mar 29, 2014
@aaronjudd
Copy link

@aaronjudd aaronjudd commented Mar 29, 2014

Going with the transform refactor... thanks guys.

@glasser glasser added Blaze labels Apr 4, 2014
@glasser
Copy link
Member

@glasser glasser commented Apr 4, 2014

@avital My personal two cents: I think the warning is nice in the case that the ID actually literally came from an _id field on an object but should be suppressed if the ID was implicit (eg a string value).

@avital
Copy link
Contributor

@avital avital commented Apr 14, 2014

I agree with @glasser's suggestion. We'll look at pull requests for this if someone wants to dive in.

@SachaG
Copy link
Contributor

@SachaG SachaG commented Jul 7, 2014

I've actually run into this problem again using {{> UI.dynamic}} inside an {{#each}} loop.

Here's the code:

{{#each modules}}
  {{> UI.dynamic template=moduleTemplate data=moduleContext}}
{{/each}}

The problem here is that the moduleContext data context also contains a MongoDB document among other properties. I want to pass the same document to each module (each module displays a different subset of the data), which triggers the warning since all documents have the same _id.

I found a workaround by storing the document in a sub-property of moduleContext instead of at the object's root, but I thought I'd still make a note of the issue.

@DenisGorbachev
Copy link
Contributor

@DenisGorbachev DenisGorbachev commented Jul 7, 2014

Same here with v0.8.2. The problem is exactly the same as described by @timtch.

@avital avital reopened this Jul 9, 2014
@stubailo stubailo added the triaged label Oct 22, 2015
@weeger
Copy link

@weeger weeger commented Dec 18, 2015

+1

@zol zol removed the backlog label May 3, 2016
@hwillson
Copy link
Member

@hwillson hwillson commented Dec 7, 2016

The original reported issue has been addressed by PR #2227. Regarding the new issue reported in #1980 (comment), if someone is able to create and link to a small repro showing this, that would be great. Due to the fact the original issue has been addressed, and given the age of this issue, I'll close this for now. Happy to re-open if anyone feels this issue should still be addressed, and an issue reproduction can be provided. Thanks!

@hwillson hwillson closed this Dec 7, 2016
@smohantyCME
Copy link

@smohantyCME smohantyCME commented Dec 17, 2019

Any status on removing the warnings?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.