Problems with Inbound primary key in some cases

[MichaelFroeschen]

I tried to use dexie in a create-react-app environment using typescript.
In this environment using an inbound primary key (++id) fails if the model class contains that key as a field. (Like shown in the typescript example in the docs)
An example of this problem can be found in this repository

As far as i can tell, this might happen because the model class actually has a field named id. (so that it can be accessed)
Without the id field, the example works.

This is reflected in the compiled code.
In the create react app example, the compiled Entry class looks like this:

class Entry {
  constructor(text) {
    this.id = void 0;
    this.text = void 0;
    this.text = text;
  }
}

In this case, the id field is explicitly defined.

In this example the compiled Entry class looks like this:

var Entry = function Entry(text) {
  (0, _classCallCheck2.default)(this, Entry);
  this.text = text;
};

In this case, the id field is not explicitly defined.
Changing this example in such a way that the id field is set to undefined in the constructor (see the Entry class in DB.ts) leads to the same issues like in the create-react-app example.

[dfahlander]

undefined is not a valid key in indexeddb. There is a difference between not having a prop set at all and to having it set to undefined. IndexedDB allows you to have auto-incremented primary keys in case you do not specify the inbound key on the object. If you do, the system will not provide the key for you. But in this case you specify the key as undefined, which is not supported.

[MichaelFroeschen]

The issue here is, that no value is specified. I just define that id as a field exists (the same thing that is done in the example from the official docs.
There should at least be a way to define the id field in such a way that dexie would not treat it this way.
Also, undefined is (IMO) not a value, but an indicator and should be treated as such.

If no such way is provided, dexie is only usable with several workarounds for about everyone using a similiar build system then cra (like using @babel/plugin-transform-typescript)

[dfahlander]

I see the issue which basically comes since JS class fields where introduced. If you transpile with typescript tsc to ES5 or ES6 you won't have this issue since a declaration will not do anything. But as babel-plugin-transform interprets the new JS spec correctly, the declaration will also mean an initializer to undefined.

What Dexie could do here is possibly to workaround this new situation when declared fields correspond to an auto-incremented key and delete it in case it is undefined before passing it to IndexedDB. But it's a good question whether it is the right thing to do. On one hand this could be to intrusive but on the other hand, it would not be nice not being able to declare the indexed fields.

Ideas and comments are welcome!

[dfahlander]

A workaround for now is probably to use merged declarations:

interface Entry {
  id: string;
  text: string;
}
class Entry {
  constructor(text: string) {
    this.text = text;
  }
}

[dfahlander]

...or how does plugin-transform-typescript handle the situation when property as optional?

class Entry {
  id?: string
  text: string
}

Will it still transpile it with a class field for id?

[MichaelFroeschen]

I agree. It could be intrusive and subsequently lead to issues for others, that actually may rely on this behaviour...

The workaround looks nice enough, i give it a try.

An optional field doesn't work (see my example repository)

Unfortunatly i do not see a solution that would not be intrusive in any way...
Thats why i thought about a specific value that could be set in order to indicate that the field is not actually initialised.
Something like NaN or something like that... But that also seems like a "dirty" solution...

[dfahlander]

This would be a problem for everyone that wants to store class instances in Dexie and have auto-incremented keys.
I came to the conclusion that this needs to be fixed. I don't want to change the docs to require people to use merged declaration just for this sake. I've fixed it now in PR #1119 and I don't think the fix is too intrusive, as it clones the given object and deletes the primary key in case it is set to undefined prior to forwarding it to add() or put().