derbyjs / racer

Realtime model synchronization engine for Node.js
1.18k stars 117 forks source link

Strange model set behaviour #257

Closed ovvn closed 3 months ago

ovvn commented 6 years ago

This line throws Uncaught Error: Referenced element not a list

model.set('_page.game.scenario.11', ['hello'])

This model.set works fine

model.set('_page.game.scenario.test', ['hello'])

If we already have ['hello'] at path '_page.game.scenario.11', then this will throw Uncaught Error: Referenced element not a list

model.set('_page.game.scenario.11', ['hello123'])

But this will work without any errors:

model.setDiffDeep('_page.game.scenario.11', ['hello123'])
balek commented 6 years ago

I can't reproduce this error. Could you give more information on data stored in model?

ovvn commented 6 years ago

Ok, the data is following:

{"game": {
  "id": "6a30efbc-741e-46ae-9c65-3b54272d58c9_new-recruit-v3",
  "scenario": {
    "11": ["hello"],
    "name": "New Recruit",
    "gameType": "new-recruit-v3",
    "gameCategory": "v3",
    "demoStrategyList": [
      "random"
    ],
    "roles": [
      "Recruiter",
      "Candidate"
    ],
    "anonymousMode": true,
    "xlsxDownload": true,

    "scenarioName": "New Recruit",
    "onlineType": "inClass",
    "rolesMeta": {
      "Recruiter": {
        "alias": "role0"
      },
      "Candidate": {
        "alias": "role1"
      }
    },
    "id": "temp_6a30efbc-741e-46ae-9c65-3b54272d58c9_new-recruit-v3",
    "_templateName": "Default",
    "base": true,
    "parentId": "a178fe18-51ae-44de-be7a-728476dbfe60"
  }
}}

I think it's important to notice that forked versions of racer and derby.js are used: racer https://github.com/dmapper/racer#sync-upstream4 derby https://github.com/dmapper/derby#sync-upstream7

balek commented 6 years ago

Avoid using numeric object keys with racer. While it's working on local docs (in upstream version), I believe ShareDB will reject them in remote docs.

ovvn commented 6 years ago

Yes I got that using numeric object keys is a bad idea, though it looks inconsistent that it works here model.setDiffDeep('_page.game.scenario.11', ['hello123']) and writes the changes to DB without errors but doesn't work this way model.set('_page.game.scenario.11', ['hello123']). And it obviously will work in this case:

model.scope('_page.game.scenario').set
  {
      "name": "New Recruit",
      "gameType": "new-recruit-v3"
      "123":
        [
          "hello",
          "world"
        ] 
   }
michael-brade commented 6 years ago

Well, the reason this model.set('_page.game.scenario.11', ['hello123']) doesn't work is that .11 makes racer interpret _page.game.scenario as array and tries to set scenario[11] to hello123. Which obviously doesn't work because _page.game.scenario is an object, hence the error. I don't know why setDiffDeep behaves differently...

michael-brade commented 6 years ago

And, btw, if it is an array, it will first resize the array to at least 11 and then write to array[11]. So trying to set a property .123 will resize the array to 123. Not a good idea...

ovvn commented 6 years ago

@michael-brade thanks for the information. Yes, we have an object with numeric keys, suppose it was set as follows:

model.scope('_page.game.scenario').set
  {
      'name': 'New Recruit'
      'gameType': 'new-recruit-v3'
      'courses':
         '11': ['hello']
         '127': ['world']
         'default': ['hola']
   }

then if I try to do model.set('_page.game.scenario.courses.127’, ['hello123’]) it will throw but model.setDiffDeep(‘_page.game.scenario.courses.127, ['hello123’]) will let me write into DB without any errors. I found it inconsistent.

craigbeck commented 3 months ago

Thanks for your contributing your issue.

We have recently been actively updating the Derby and Racer packages, and both repos are now in Typescript and published with types on npm. As we have quite a few old issues open we have made the decision to close out of date issues.

If this issue still matters to you we encourage reproducing against the current versions of the repo and opening a new issue.