medikoo / dbjs Goto Github PK

That way, we can take advantage of it when working with any type of observables, and not necessarily using dbjs

We should throw meaningful error message if someone wants to assign type from different database realm

/cc @kamsi

Currently it is allowed to tweak descriptor characteristics directly on descriptor objects.
It is error prone, as in some cases we may do it not being aware that descriptor presents not desired context e.g. following looks innocent

obj.getDescriptor('foo').type = db.X;

However as getDescriptor was used and not getOwnDescriptor, it may appear that we extend definition for way broader range than obj (which looks as intended context). This may lead to difficult to track bugs.

I think the only solid solution to prevent such issues, is to restrict definitions so at all times context needs to be indicated.
So definition will be allowed only via define (defineProperties) or defineProperties, and then we may introduce extendProperty or extendProperties when we want to extend definition of already defined property.

Order should reflect order of resolved value, even if it has changed over time

As conceptually it's about Object.assign

/cc @kamsi

Currently it's provided with dbjs-ext, however it's very popular functionality, and additionally it'll be great to have possibility to define enum characteristics directly in property definitions (so on descriptors)

/cc @proti

Old value is being resolved on already turned object, and in that case state of all other property values is no longer reliable. That leads to crashes in getters

/cc @kamsi @nix1

Currenlty by default sets and maps are iterated by last modification order. For most cases it plays well, still it might be wise to expose compare method which can be overriden for certain use cases

e.g. we may have list of countries, that we want to be iterated alphabetically, we can define them that way in model, but what if want to assure alphabetical order in any language (where labels will be different for each language), with custom compare method, it'll be possible to assure universal alphabetical order for such type.

/cc @kamsi

We should have easy means to get deep properties via keyPath. At this point there's obj.resolveSKeyPath method that can be used for that, but it's not as convenient and intuitive as it can be.

it would be nice to be able to call toJSON() on an object in the database in order to debug, test, or send over a traditional HTTP API. is this friendly for in dbjs, or is dbjs general enough that this should be in es6-map and es6-set? i'd be happy to write the code.

cheers!

Currently we have define and defineProperties.

/cc @mtuchowski @kamsi

It might be good to bring in new version of dbjs, following change:

master -> owner
owner -> parent

would be especially useful in the context of #3.

/cc @kamsi

Occasionally we try to use variables from out of function scope, that should not be the case, as getters are serialized and unserialized, so scope is lost in execution on client-side.

As reported by @roxanadina:

when doing

john.doctor = drHouse

$ node test.js

DbjsError: Some values are invalid:
       firstName,John is not a Patient
lastName,Smith is not a Patient
birthDate,Mon Jan 03 1977 00:00:00 GMT+0200 (GTB Standard Time) is not a Patient

   at defineProperties._validateMultiple_ (d:\workspace\node_modules\dbjs\_setu
p\1.property\4.descriptor-validate-value.js:129:10)
   at defineProperties._validateSetValue_ (d:\workspace\node_modules\dbjs\_setu
p\1.property\4.descriptor-validate-value.js:53:35)
   at Self.defineProperties._validateSet_ (d:\workspace\node_modules\dbjs\_setu
p\1.property\7.set-property.js:190:16)
   at Object.create._validateSet_ (d:\workspace\node_modules\dbjs\_setup\1.prop
erty\reverse-map.js:155:9)
   at defineProperties._validateSetValue_ (d:\workspace\node_modules\dbjs\_setu
p\1.property\4.descriptor-validate-value.js:47:46)
   at Self.defineProperties._validateSet_ (d:\workspace\node_modules\dbjs\_setu
p\1.property\7.set-property.js:190:16)
   at Self.<anonymous> (d:\workspace\node_modules\dbjs\_setup\1.property\8.acce
ss-property.js:32:26)
   at Object.<anonymous> (d:\workspace\newGit\dbjs-new\test.js:28:13)
   at Module._compile (module.js:456:26)
   at Object.Module._extensions..js (module.js:474:10)


when doing

john._set_('doctor', drHouse)

d:\workspace\node_modules\dbjs\_setup\1.property\reverse-map.js:145
               if (set.has(value)) return;
                       ^
TypeError: Cannot call method 'has' of undefined
   at Object.create._set_ (d:\workspace\node_modules\dbjs\_setup\1.property\re
erse-map.js:145:11)
   at Self.defineProperties._set_ (d:\workspace\node_modules\dbjs\_setup\1.pro
erty\7.set-property.js:82:20)
   at Object.<anonymous> (d:\workspace\newGit\dbjs-new\test.js:29:6)
   at Module._compile (module.js:456:26)
   at Object.Module._extensions..js (module.js:474:10)
   at Module.load (module.js:356:32)
   at Function.Module._load (module.js:312:12)
   at Function.Module.runMain (module.js:497:10)
   at startup (node.js:119:16)
   at node.js:906:3

Computables should be set only through define and newly introduced setComputable

We may also think of restricting regular set of property if value on underlying prototype is computable

/cc @kamsi @mtuchowski

Currently, Set is defined via multiple: true, and Map (when we think of object with each property of same type) via _desciptorPrototype_.setProperties({ type: Object, nested: true }).

It doesn't seem intuitive, and probably can be improved, by removing both multiple: true and descriptorPrototype functions, and instead introducing aside of type meta property, an itemType property

So sets can be defined as:

Class.prototype.defineProperty('someSet', { type: db.Set, itemType: db.String })

and maps as e.g.:

Class.prototype.defineProperty('someMap', { type: db.Object, itemType: db.String })

Additionally for object kind of itemType's we may introduce (optional) itemPrototype property, which will allow to customise type provided for itemType without a need to create such type as standalone one. That thing is not possible in current implementation of dbjs (all we have is _descriptorPrototype_)

It's in primitives basket, having just this one type a methods that allow to change self value is out of convention

/cc @kamsi

Currently on incoming data batch, following happens:

1. Static (non-computable) records are put in place. If some record changes state from non computable to computable (may happen if some object got back from destroyed state to I'm here again state), it's also immediately recomputed.
2. All computables that depend on statics are recomputed
3. All computables that depend on computables are recomputed (and so on)
4. Events for boths statics and computables are propagated

There's an issue with resolution of new computables in first step, there's a risk they may approach uneven state. Technically that should be put between step 1 and 2.

At this point I've introduced a fix that forces another re-computation of them in step 2.

If for property of type Function, function instance is passed, and it's neither direct instance of this Function type, nor plain JavaScript function, validator should throw.

In case of Dates or RegExp's validator instead of throwing may create copies, but in case of functions there's no straightforward way to create perfect copy.

It will prevent errors, as one we approached with @kamsi, where following bogus code:

db2.Type.extend('SomeType', db1.OtherType);

didn't report any issue, but have broken state of db1.OtherType (its prototype was turned)

/cc @nix1

Currently when we try to define a property using type from other database instance, we get very basic error message: "X is not a valid type". It'll be nicer to indicate to developer that we can't use foreign type.

/cc @kamsi

Currently reverse resolution has priority, even if on extended class we want to shadow property resolved by reverse with a getter, it is not allowed.

It's plainly because validation of reverse gets priority, and crashes definition stating it received unexpected value.

Additionally it would be good to provide separation between definition of value, and set of value.
Currenlty if value is set via define, it is put through normal set validation, where source of action cannot be distinguished.

if i wanted to implement something like a feathers service or dstore/Store with find / get / create / update / remove functions for each data type, what is the most idiomatic way to do this?

• find(params) maybe uses db.objects.filterByKey(key, value) for each keyvalue pair, any other pointers?
• get(id, params) is db.objects.getById(id).
• create(data, params) is db.Type(data). what about nested objects?
• update(id, data, params) is a get(id) then obj[key] = value for each keyvalue pair. nested objects seem fine here.
• remove(id, params) is db.objects.destroy(id). what about nested objects?

then to forward observe events, we listen with db.objects.on('change', ..., but how do we access the Type of the changed object?

cheers!

Currently generic function string is genereted, and doesn't bring any meaningful information. It should be similar to db object instances, e.g. [dbjs String]

/cc @kamsi

At this point all defined properties, are set as enumerable. There's no way to tweak that.

There are however use cases, when we may want non-enumerable property on an object. Good example is map of same type values, where we want to add some custom property, that should not be treated as part of map collection, and should not be iterable with forEach.

There's a specific use case for that in eRegistrations app, where we have map of sections, and we need a property that will calculate overall status of the sections. At this point we put it aside on an object as sectionsStatus, while more natural would be to have it as sections.status.

@medikoo was kind enough to share an engine agnostic example of persistent binding. :)

/cc @kamsi

Currently all we can provide is a key while sometimes it can be valuable to to pass it to nested property.

We would need to address it with another property, probably reverseKeyPath and maybe rename reverse to reverseKey which would also be more self explanatory

/cc @kamsi @mtuchowski

Now each class is set directly on dbjs, so they're names need to be quite specific (as they may collide with other not related class names), e.g. we may have db.Attorney and db.AttorneyFormSection classes.

However it might be to good to have possiblity to narrow class name to some namespace. so it's e.g. db.Attorney, db.FormSection and db.FormSection.Attorney.

In such cases we may also require that nested class extends parent class (so e.g. in above case it's clear that db.FormSection.Attorney extends db.FormSection)

/cc @mtuchowski

e.g. say that foo.bar should return same thing as foo.lorem return.

Currently it's achievable via getters, but if we deal with sets (multiple values) then always a reactive copy of resolved set is returned, and not set itself, which is unnecessary overload for some scenarios

Currently it needs to be manually retrieved, and due to specific observables resolution requires additional specific hacks as:

// some getter
value: function (_observe) {
  var superGetter = this.database.SuperClass.getDescriptor('property')._value_;
  superGetter = this.database.resolveGetterObservables(superGetter);
  var superResult = superGetter.call(this, _observe);
}

While it possibly might be as easy as:

// some getter
value: function (_super) {
  var superResult = _super();
}

/cc @kamsi @mtuchowski

Currently there's $ prefix for descriptors and _ for observables.

Both seem controversial, and to uninitiated developer they provide just wrong clues on what's behind (e.g. $ may suggest some jQuery call, while _ a private property).

Initial proposal is to introduce d and o methods, So instead of obj.$foo or obj._foo, developer may use obj.d('foo') and obj.o('foo').

Better proposals are welcome

It must not be allowed to have it defined twice for same type with same reverse property name.

Lack of this validation produced serious bug in Salvador system

Normally application should work with its database working in read-only mode.

Where data needs to be updated (e.g. after form submission), read-only should be postponed for a moment of update with special handler, e.g. it may work as:

db.write(function () {
  user.firstName = "John";
  ...
});

This will prevent developers from accidentally (or not accidentally) introducing updates to database.
Currently any update of db data anywhere in a code, will just work without a warning, that's not great.

It will also be a great stopper, from not wise ideas of introducing updates within property getters.

/cc @kamsi

After upcoming refactor of dbjs, it'll be great so it works naturally with typical REST server backed (assuming that we want to use engine just on client side).

For that it'll be good if dbjs database instance can be easily updated via incoming JSON objects, and that we have possibility to get JSON snapshots of chosen data from dbjs database.

Currently such setup, while possible, is not straightforward and demands custom configuration.

/cc @kamsi

Naturally we can have map of sets, map of maps, but there's no way to have set of sets. Use cases are rare, but they exist, and it'll be great to have it possible.

/cc @kamsi

Definitions accept strictly descriptors, when by mistake sometimes we try to pass value directly.

It would be good to throw on obviously non descriptor objects, whether we should accept just plain objects is questionable, but maybe it's the way it should be done.

Currently when property is defined as nested, value (nested object ) becomes uncoditionally set. There's no way to introduce null alternative for such property.
There are many use cases where such approach becomes problematic.

Serialization of such state might go to 7 when nested should be returned.

/cc @kamsi

Currently when we want to configure some action on when given object receives certain state, we usually build collection as e.g.

db.BusinessProcess.filterByKey('isRevisionReady', true).on(function (event) { ... });

What's painful is that usually we rewrite similar event handler which works around add, delete, batch events. It might be nicer if we can achieve same via something as:

db.BusinessProcess.stream('isRevisionReady', true).on(function (businessProcess) { .. });

This stream will also at initialization stream all objects that already match expected state.

Additional functionality would be to create a streams that are also guarded by pre-triggers, so we observe specific state change from A to B, and not just fact that object landed at given state.
Very rough idea, on how it may look:

db.BusinessProcess.stream('isRevisionReady', true)
  .to('isRevisionApproved', true).on(function (businessProcess) { .. });

Currently when property is defined as nested, it's not possible to override with remote object.

It should be possible, there are cases when we may want property to be either nested or remote object.
Good use case is an image file, with preview property. In some cases preview may link self image file. Currently if we define preview as nested it can't work properly

/cc @kamsi

It was in some older version of dbjs, but hardly used. Still it might be worth to bring it back.
We should also introduce obj.isValid(key), and we should also consider support for requireOwn

/cc @kamsi

There's most likely no use case to use timestamps from future, and when they're accidentally injected, they introduce hard to investigate bugs.

It might be good to throw whenever such timestamp is proposed

As noticed with @kamsi, behavior is not the same as with regular new Type()

/cc @kamsi

Currently when doing obj.getDescriptor('x') we'll always get some descriptor. If property was never defined, we'll get base descriptor that is ancestor for each descriptor in a database (a base descriptor prototype)

It's dangerous, as when we try to do obj.getDescriptor('x').someSetting = value we may accidentally set this value for every property in a system.

Currently as an alternative there's obj.getOwnDescriptor('x') which by all means will return descriptor for given object (if it didn't exist, it's created and returned). Still it's dirty, as it creates objects we may not need. It's also usually not used, as in most cases we call getDescriptor on instances to get descriptor of prototype properties.

I see three possible solutions at the moment.

1. Make getOwnDescriptor (or create alternative method) so it returns own descriptor only if it exists, and returns null otherwise. It will quickly expose eventual bugs.
2. Do not return base descriptor (of $ id), from getDescriptor call, so it's accessible via public API.
3. Freeze base descriptor (of $ id) (via Object.freeze) so any changes on it are forbidden.

/cc @kamsi

There are cases, when we want to define property, to which other type may be assigned.
Currently at definition step to make it work properly we need to leave db.Base type as type, which technically allows all supported dbjs values to be set, so there's no constraint for Type objects, as it should be.

Additionally allow to restrict given type property to receive only type extensions of specific type

/cc @kamsi @mtuchowski

Sometimes we try to set properties directly in dbjs via obj.foo = 'bar', but if property of given name was never defined in dbjs (on any object), then it doesn't reach dbjs getter but becomes natural ECMAScript property set.

This behavior leads to issues, as our settings may get lost, when we do not expect it.

It might be good to try to find possibly dev only solution that would warn us about such usage (or even throw). It might be possible in ES6 with usage of proxies, or with Object.observe if V8 implements it.

Currently the flow when we want to reactively display value in DOM is that we have some DOM handler which observes value directly and then on each change it stringifies value and updates DOM with it.

While it should rather be, that we observe already stringified version of that value, and on its change we update DOM.

So all goes to point that there should be an easy way to observe results of toString for given property

/cc @mtuchowski @kamsi

e.g. we'd like to have just upper case result. This case is normalizable, while in dbjs all we have now is pattern which is treated strict way (if string doesn't match it is assumed invalid and cannot be normalized), Still pattern would not be ideal for upper, lower case as it's not possible to easily and accurately describe such rule with regex.

Summarising this is about two things:

• Allow ease configuration of forced case (lower or upper) on string types and string property descriptors
• Alllow custom normalisation for pattern validation, for feasible cases (e.g. entering - betweet NIP numbers), in such case even if input string doesn't match pattern it may be treated as normalisable, then valid.

/cc @kamsi

There are some of them, which can't be defined successfully like owner, master, id

Currently dbjs allows definition of them, but resolution of them of course doesn't work as expected.
Best would be to throw at definition moment

/cc @kamsi

So e.g. doing obj.keyPath for object of id 3sdfaf3223/foo/bar we get foo/bar.

Values for properties with no type will never be validated, and they won't be propagated to persistent layer.

Valid use case for that, is to have computables that do not necessarily return values that are applicable for persistent layer. Currently it's not possible