Montage data converters#1 - Githubissues

montagejs / montage

Montage is an elegant, open source HTML5 framework maintained by Montage Studio that rivals native SDKs, yet is easier to learn. It offers modular components, two-way data binding, and much more. Join us on irc.freenode.net#montage. Sign up for our beta to build Montage applications in the cloud.

http://montagestudio.com/montagejs

Other

1.5k stars 215 forks source link

Montage data converters#1 #2010

Closed marchant closed 4 years ago

marchant commented 5 years ago

To merge after /feature/npm3

Adds a new new converter to map embedded types
fixes a deserialization bug
adds resolveObjectForTypeRawData method to RawDataService

marchant commented 5 years ago

Agreed, how about parent: RawValueToObjectConverter then RawEmbeddedValueToObjectConverter and 1. RawForeignValueToObjectConverter ?

marchant commented 5 years ago

refactor/add convertDataQuery and revertDataQuery to adds more transparency and allow upper layers to expose that query.

marchant commented 5 years ago

This merged the latest in #master to get sync capabilities in the deserializer, and it needs https://github.com/montagejs/mr/pull/146 to work, which is itself additional to the npm3 work, so both PR go hand in hand

marchant commented 5 years ago

montage tests pass, also tested with a montage data improved version of shop-demo (which wasn't working without these changes as it's the first use of a main.datareel folder that contains all data related assets) as well as Popcorn master. Please review again.

marchant commented 5 years ago

Some test fails as it needs https://github.com/montagejs/mr/pull/146, I didn't change package.json, so it still points to "18.0.0-rc1" where https://github.com/montagejs/mr/pull/146 needs to be merged on

tejaede commented 5 years ago

An error with DataService deserialization bubbled up some inconsistencies.

DataService._childServices is designed to be a Set, per the documentation. However, DataService.deserializeSelf set the value to an array.
DataService.deserializedFromSerialization called DataService.addChildServices with DataService._childServices. As mentioned above, DataService._childServices is an Array if the serialization includes child services, but is a Set if the value is allowed to lazy load.

I am unable to write to marchant/montagejs so I pushed fixes to my own repo for these issues for your review: https://github.com/tejaede/montage/commit/982991277699a6f9280ee27995647cdab3686b7f

The fixes are:

Ensure DataService.childServices is always a set
Use iterator in DataService.addChildServices so the argument can be either a Set or an Array

cc: @marchant

marchant commented 4 years ago

There's a problem running karma on travis, we need to fix this independently of this PR, merging.