stampit-org / stampit

OOP is better with stamps: Composable object factories.
https://stampit.js.org
MIT License
3.02k stars 103 forks source link

when i use getter and setter, i got this, is this a issue? #344

Closed lornally closed 5 years ago

lornally commented 5 years ago

maybe this is issue, or maybe i use it by incorrect method, here is my code.

function getset() {
  const x = stampit()
    .props({ i: 'iiiii' })
    .methods({
      get info() {
        return `get${this.i}`;
      },
      set info(info) {
        this.i += `hahah${info}`;
      },
    });

  const t = x();
  t.info = 'ttttt';
  log('info:', t.info); //now  t.info is 'ttttt'  !!!!!! , it look like getter and setter is useless?
  log('i:', t.i);
}
koresar commented 5 years ago

Hello there! Thank you for the issue. This is an unimplemented feature. It is requested quite often. I guess it's time to implement it. Sorry about that. I need to allocate some time to make it happen.

On Tue., 4 Jun. 2019, 06:19 茂弘, notifications@github.com wrote:

maybe this is issue, or maybe i use it by incorrect method, here is my code.

function getset() { const x = stampit() .props({ i: 'iiiii' }) .methods({ get info() { return get${this.i}; }, set info(info) { this.i += hahah${info}; }, });

const t = x(); t.info = 'ttttt'; log('info:', t.info); //now t.info is 'ttttt' !!!!!! , it look like getter and setter is useless? log('i:', t.i); }

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/stampit-org/stampit/issues/344?email_source=notifications&email_token=AAMMEL3NJNJA7QFAOULH7BLPYXNMFA5CNFSM4HSYBPYKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GXNTE6Q, or mute the thread https://github.com/notifications/unsubscribe-auth/AAMMEL23EUNMUG2RA7DN2BDPYXNMFANCNFSM4HSYBPYA .

lornally commented 5 years ago

good, and maybe i can help you. :)

koresar commented 5 years ago

Superb! The code is very unreadable though. The getters and setters should be copied to the ".methods" object while composing.

On Tue., 4 Jun. 2019, 09:59 茂弘, notifications@github.com wrote:

good, and maybe i can help you. :)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/stampit-org/stampit/issues/344?email_source=notifications&email_token=AAMMEL6QXX6WYKMAD4VJEC3PYYHETA5CNFSM4HSYBPYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODW3TTHA#issuecomment-498547100, or mute the thread https://github.com/notifications/unsubscribe-auth/AAMMEL3VO3HDZIO64RF6YFLPYYHETANCNFSM4HSYBPYA .

lornally commented 5 years ago

yes, it is really tough, i am trying to understand it.......

koresar commented 5 years ago

Here is almost the same code but more readable. https://github.com/stampit-org/stamp-specification/blob/master/compose.js

On Wed., 5 Jun. 2019, 05:32 茂弘, notifications@github.com wrote:

yes, it is really tough, i am trying to understand it.......

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/stampit-org/stampit/issues/344?email_source=notifications&email_token=AAMMEL3RQ3HYDPHJVU72XV3PY4QT5A5CNFSM4HSYBPYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODW6NSHQ#issuecomment-498915614, or mute the thread https://github.com/notifications/unsubscribe-auth/AAMMEL6KELHJ7ZW22HKCOYLPY4QT5ANCNFSM4HSYBPYA .

lornally commented 5 years ago

the code is really elegant, but i really can not understand why getter/setter is not work, the stampit copy all property of the object. but why getter/setter is not work? maybe i need to get more information about getter/setter.

just debug some test code, i find the problem:

// if we build a object by stamp , 
__proto__: {__proto__:  {__definegetter__:function, __lookupgetter__:function} }

//if we build a original object,
__proto__:  {__definegetter__:function, __lookupgetter__:function} 

because we make a nesting proto, so can not use getter/setter, can we remove the proto?

lornally commented 5 years ago

my error, this code is ok, so the proto nest is not problem:

class X {
  constructor(name) {
    this._name = name;
  }
  get name() {
    return this._name;
  }
  set name(name) {
    this._name = `${name}X`;
  }
}
class Y extends X {}
const yy = new Y('');
yy.name = 'hi';
log('yyyyyyy', yy); //
lornally commented 5 years ago

maybe i have find the solution, because i find a big lack of my cognitive concept.

so we can do like this:

  1. Object.getOwnPropertyDescriptors() or Object.getOwnPropertyNames()
  2. then Object​.define​Properties()

this code can run in right way:

const yya = stampit().compose({
    properties: {
    i: 'iiiii',
    get info() {
          return `get${this.i}`;
        },
    },
});
const yyya=yya();
log('yyya info2:', yyya.info);  // return: 'getiiiiii', that is right.

but now setter is not run in right way.

koresar commented 5 years ago

Sounds you're on the right track! Please, continue. 👍

On Mon., 10 Jun. 2019, 09:25 茂弘, notifications@github.com wrote:

maybe i have find the solution, because i find a big lack of my cognitive concept.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/stampit-org/stampit/issues/344?email_source=notifications&email_token=AAMMEL4WLURPSAYMSGE7RALPZXXT5A5CNFSM4HSYBPYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXJBA6Q#issuecomment-500306042, or mute the thread https://github.com/notifications/unsubscribe-auth/AAMMEL6DSEJSYSGUAWVLS23PZXXT5ANCNFSM4HSYBPYA .

lornally commented 5 years ago

i got it, i know why getter ok, but setter in not run, because we use assign, we need a alternative solution:

const y = {
  i: '-init-',
  get info() {
    return `read${this.i}`;
  },
  set info(info) {
    this.i += `write${info}`;
  },
};
log('0object0 info:', y.info);  //right
y.info = 'y';
log('1object1 info:', y.info);  //right too

//test create
let x=Object.create(y);
log('00x info:', x.info);  //pass
x.info='x'
log('11x info:', x.info); //pass
log('11x11 info:', x.pp());  //pass

//test assign
x=Object.assign({}, y);
log('22x info:', x.info);  //pass
x.info='x'
log('33x info:', x.info);  // here is the error, setter cannot run. so assign is the reason .
log('44x info:', x.pp());  //pass

//test spread...
x={...y};
log('55x info:', x.info);
x.info='x'
log('66x info:', x.info);  //here error.
log('77x info:', x.pp());

so we can use create to init the object, but how can we compose it? i need try more......

lornally commented 5 years ago

this is the right code, maybe, i need change a mass of code, every property include deep and static:

 function compose(obj) {
    return function com() {
      const p = Object.defineProperties({}, Object.getOwnPropertyDescriptors(obj));
      return p;
    };
  }
lornally commented 5 years ago

this is demo: com=compose

export function com(obj) {
    let ob =this? this.ob || {}:{};
    ob=Object.defineProperties(ob, Object.getOwnPropertyDescriptors(obj));
    const xx=function xx() {
        return Object.create(ob);
    }
    xx.com=com;
    xx.ob=ob;
        return xx;
}

we need avoid use assign, ...spread, for/for in/foreach, almost all of array.prototype, we need change all properties clone code. maybe just like refactor.

lornally commented 5 years ago

i have fix it in stampit-org/stamp-specification, but i can not fix it in stampit, because cannot pass the test. this is the code for stampit-org/stamp-specification:

// const assign = Object.assign; fix getter/setter bug.
const assign = (ob, obj) => {
  if (typeof obj !== 'undefined') {
    return Object.defineProperties(ob, Object.getOwnPropertyDescriptors(obj));
  }
  return ob;
};
koresar commented 5 years ago

Stampit has a set of very robust tests. Let me check why this doesn't work.

lornally commented 5 years ago

meybe i know why, because object.assign(a, b1, b2,b3....), maybe i need compatible with it. , i will make a PR, but some test can not pass still, sorry....:`(

AHaliq commented 5 years ago

hows progress with the setter? I'm trying to use getters and setters with stampit too.

koresar commented 5 years ago

Going well. Thank you for poking. 😀 The coding bit is done. The unit tests left. Sorry for the delay. (I'm having a baby girl soon.)

lornally commented 5 years ago

congratulations :)

koresar commented 5 years ago

Thank you 😀

koresar commented 5 years ago

Implemented and published as stampit v4.3

koresar commented 5 years ago

@lornally @AHaliq thank you people for poking me from time to time. :) The feature is your merit.