These are chat archives for canjs/canjs

5th
Jun 2017
Brad Momberger
@bmomberger-bitovi
Jun 05 2017 13:49
@gregorgodoy a common pattern we use in the model layer is to create the model definition as a subclass of DefineMap, and add the connection that maps to it as a static property on the class (we usually use the property name Connection with a capital C. Then you can explicitly call CountryModel.Connection.createData(this.serialize(), this._cid)
The other option is to override isNew() and use some other criteria than the presence of the idProp to determine whether to create or update. How will you know whether an object has been created yet or not?
gregorgodoy
@gregorgodoy
Jun 05 2017 16:49

@bmomberger-bitovi thanks for your reply and suggestions! I tried your suggestion and is almost working. For sure i misunderstood something. Here the model:

import DefineMap from 'can-define/map/';
import DefineList from 'can-define/list/';
import set from 'can-set';
import superMap from 'can-connect/can/super-map/';
import loader from '@loader';

const Multimedia = DefineMap.extend({
    seal: true
}, {
    'filename': 'string',
    'mime': 'string',
    'size': 'string',
    'original_name': 'string'
});

const algebra = new set.Algebra(
    set.props.id('filename')
);

Multimedia.List = DefineList.extend({
    '#': Multimedia
});

Multimedia.connection = superMap({
    url: {
        createData: function(param) {
            return new Promise(function(resolve,reject) {
                $.ajax({
                    url: loader.serviceBaseURL + '/multimedia',
                    type: 'POST',
                    dataType: 'json',
                    data: param,
                    headers: {
                        Prefer: 'return=representation',
                        Accept: 'application/vnd.pgrst.object+json'
                    }
                }).then(resolve, reject);
            });
        }

    },
    Map: Multimedia,
    List: Multimedia.List,
    name: 'multimedia',
    algebra
});

export default Multimedia;

and then i tried the following:

model =  new Multimedia({
    filename: '131232-2131231-23123.jpg',
    mime: 'image/jpeg',
    size: '1243',
    original_name: 'portrait.jpg'
})
model.constructor.connection.createData(model.serialize(), model._cid);

And the createData is called!! And the model is persisted in the API. GREAT!! But, immediately donejs throws an error:

Potentially unhandled rejection [2] TypeError: Cannot use 'in' operator to search for '__get' in undefined at readObservabe (http://baker:81/node_modules/can-connect/can/map/map.js:746:13)

and the exactly line is:

var readObservabe = function(instance, prop){
    if("__get" in instance) {
        if(callCanReadingOnIdRead) {
            Observation.add(instance, prop);
        }
        return instance.__get(prop);
    } else {
        if(callCanReadingOnIdRead) {
            return instance[prop];
        } else {
            return Observation.ignore(function(){
                return instance[prop];
            })();
        }
    }
};
gregorgodoy
@gregorgodoy
Jun 05 2017 17:13

Before i read your answer i tried the second option. I added a "flag" property to the model instance, like this:

model =  new Multimedia({
                    filename: data.filename,
                    mime: data.mime,
                    size: data.size,
                    original_name: data.original_name
                });
model._data._new = true;

And then i overwrote the save method from can-connect/constructor/constructor.js

save: function(instance){
            var serialized = this.serializeInstance(instance);
            var id = this.id(instance);
            var self = this;
            //if(id === undefined) {
            if (id === undefined || instance._data._new) {
                // If `id` is undefined, we are creating this instance.
                // It should be given a local id and temporarily added to the cidStore
                // so other hooks can get back the instance that's being created.
                var cid = this._cid++;
                this.cidStore.addReference(cid, instance);

                // Call the data layer.
                // If the data returned is undefined, don't call `createdInstance`
                return this.createData(serialized, cid).then(function(data){
                    // if undefined is returned, this can't be created, or someone has taken care of it
                    if(data !== undefined) {
                        self.createdInstance(instance, data);
                    }
                    self.cidStore.deleteReference(cid, instance);
                    return instance;
                });
            } else {
                return this.updateData(serialized).then(function(data){
                    if(data !== undefined) {
                        self.updatedInstance(instance, data);
                    }
                    return instance;
                });
            }
        }

Note i added the condition if (id === undefined || instance._data._new) { and in the save callback i set the _new flag to false. So far its working, but i think your suggestion is more elegant! What would you suggest / recommend? THANKS!

Brad Momberger
@bmomberger-bitovi
Jun 05 2017 17:16
Usually that error is thrown when the returned data from a service call isn't an object when parsed from JSON. An example being if you returned a string as a status code.
gregorgodoy
@gregorgodoy
Jun 05 2017 17:26
thanks! I checked the returned data and it is a well formed json. But i will keep tracing the error. thanks!! Did you see my alternative solution?
Brad Momberger
@bmomberger-bitovi
Jun 05 2017 17:41
Yes, I looked it over and am thinking about it for a moment.
Assuming that filename serves as a unique ID for your Multimedia instances, I would first set idProp in your connection options to be "filename", then I would override id() in your connection instead of save():
Brad Momberger
@bmomberger-bitovi
Jun 05 2017 17:47
var configuredIdConnection = function(baseConnection) {
  return {
    id: function(instance) {
      if(instance._data._new) {
        return undefined;
      } else {
        return baseConnection.id(instance);
     }
   }
  };
};
then instead of superMap, make your own connection with the behaviors that superMap uses, and tack that function on the end as the last one.
gregorgodoy
@gregorgodoy
Jun 05 2017 18:11
Thanks for the suggestions, i will try it! It looks a very elegant solution. Yes, in the example filename is the unique ID, because the data comes from another API. Maybe not the best example, but the API uses a lot of natural key, and compound natural keys. I will try it and come back soon, thanks again!
Brad Momberger
@bmomberger-bitovi
Jun 05 2017 18:13
If you have compound natural keys, you should be using set algebras to define them. looks like you're doing that. Where I said to set idProp in my previous suggestions, just be sure your set algebras are correct. sorry for missing that.
gregorgodoy
@gregorgodoy
Jun 05 2017 18:19
Great, undestood! I come back soon! Thanks for sharing your time :clap:
Dovid Bleier
@dbleier
Jun 05 2017 19:04
@bmomberger-bitovi @justinbmeyer thanks for your suggestions at https://gitter.im/canjs/canjs?at=592d95d50a783b6c0aecc10e
https://gitter.im/canjs/canjs?at=592ef39c0ba4d59763fbb6e4 (I have been out for a few days)
in the end I overwrote getListData() and updateListData on the cacheConnection
now I am faced with another serious issue. I am currently working on an upgrade to canjs 3.0
I have this code
<can-import from="cms/sign-controls/" />
<sign-controls {user}="user" {role}="role" {rolelevel}="rolelevel" {^selected-presentation}="*presentation" {^sign}="sign"></sign-controls>

<can-import from="cms/presentation-workspace/" />
<presentation-workspace {presentation}="*presentation"></presentation-workspace>
which works fine in can 2.x
the presentation is selected in the sign-controls component and passed to the presentation-workspace component where it updates a bunch of subcomponents
the problem is when I select a presentation, it does update the presentation-workspace but then creates such a memory leak, it freezes the tab and I have to kill the tab
Brad Momberger
@bmomberger-bitovi
Jun 05 2017 19:08
What version of CanJS are you working from?
Dovid Bleier
@dbleier
Jun 05 2017 19:08
this is on can@3
Brad Momberger
@bmomberger-bitovi
Jun 05 2017 19:09
Check your minor/patch versions as well. The latest CanJS is 3.8.2 and I know that we've been addressing several memory leaks recently.
Thomas Sieverding
@Bajix
Jun 05 2017 19:09
npm ls | grep can
Dovid Bleier
@dbleier
Jun 05 2017 19:11
I am running 3.6
I'll try upgrading to 3.8 and see if that does the job
Brad Momberger
@bmomberger-bitovi
Jun 05 2017 19:11
Please report back if you have issues.
Thomas Sieverding
@Bajix
Jun 05 2017 19:12
Perhaps could be some sort of merge behavior creating unintended consequences?
Dovid Bleier
@dbleier
Jun 05 2017 19:12
ok. which packages should I update? just can?
@Bajix what do you mean?
Thomas Sieverding
@Bajix
Jun 05 2017 19:12
you should upgrade all can-*
Dovid Bleier
@dbleier
Jun 05 2017 19:13
ok, is there a single command to do all that or I need to update each one separately?
Thomas Sieverding
@Bajix
Jun 05 2017 19:14
It was a while back, but there used to be some issues with how exporting state worked that could result in unsafe merges
I’m not sure what the present state of affairs there is
but there are behaviors now to address this sort of issue
Brad Momberger
@bmomberger-bitovi
Jun 05 2017 19:17
The monolithic can package fixes its dependencies at particular patches. If you're importing can in your scripts and only using that, just the one is fine while you're developing (and you may want to fix all your dependencies when packaging for production later)
Thomas Sieverding
@Bajix
Jun 05 2017 19:17
@dbleier read this: canjs/canjs#2314
Dovid Bleier
@dbleier
Jun 05 2017 19:20
@bmomberger-bitovi I have already updated my modules to use the different can- packages instead of just the monolithic can
Brad Momberger
@bmomberger-bitovi
Jun 05 2017 19:22
OK. They should all move to the correct versions if you are using ^ notation for the individual packages, but you may encounter cases where multiple versions of can-observation or can-cid are included (which isn't allowed because they have internal state)
if that happens, try deleting node_modules and reinstalling
Thomas Sieverding
@Bajix
Jun 05 2017 19:23
npm install -g npm-check-updates
That’ll make it much easier to upgrade
npm ls | grep can should all be deduped
Brad Momberger
@bmomberger-bitovi
Jun 05 2017 19:24
:point_up:
Dovid Bleier
@dbleier
Jun 05 2017 19:27
@Bajix do I really want -g if I am only working on upgrading this one project?
also tried just upgrading can and still getting the same error
@bmomberger-bitovi where would I see if multiple versions of can-observation and can-cid are included? in my package.json?
Brad Momberger
@bmomberger-bitovi
Jun 05 2017 19:31
npm ls can-observation for example.
It will show you everywhere the package tree contains it.
Dovid Bleier
@dbleier
Jun 05 2017 19:32
aha
3.15 and 3.1.3
Thomas Sieverding
@Bajix
Jun 05 2017 19:34
@dbleier you need the -g flag on npm-check-updates for it to setup the bin symlinking. That gives you a bash command, ncu, that will help update package.json files
Dovid Bleier
@dbleier
Jun 05 2017 19:35
ok
updated can-observation to 3.1.5, still not working
:(
Dovid Bleier
@dbleier
Jun 05 2017 19:40
I am even sure how to track this down. Every time I try a profiling tool in devtools, the page freezes and I have to kill it and then lose the profiling data
only thing I can think of, is remove the sub-components and readd one by one until I find the one where the leak is happening
fyi, I am just trying to get the minimum upgrade working now, so still using can-map and not DefineMap
Dovid Bleier
@dbleier
Jun 05 2017 20:01
good news, I figured out which component is causing the error, so I'll continue to track it down
@Bajix @bmomberger-bitovi thanks for you help
Brad Momberger
@bmomberger-bitovi
Jun 05 2017 20:02
Let us know if the root cause is something we can help you with
Dovid Bleier
@dbleier
Jun 05 2017 20:02
ok, thanks
gregorgodoy
@gregorgodoy
Jun 05 2017 22:49
@bmomberger-bitovi I tried to apply your suggestion. What i did so far is copy superMap and create a file called mySuperMap. I pretend to put all the customizations that are common between the models, so i will not need to specify the same on each model (for example the API needs some special headers). Then all my model will pass the options to mySuperMap instead to superMap. At the end i paste you code. The problem is im newbie to donejs, so i dont know what happens "behind the scenes" and im sure i should not just paste the code. So, first, thanks for the patient, but could you show me the example of how should it be done? Oh, and sorry for the poor english, its a foreign language for me!