New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ES6 class syntax with child tags #1451

Closed
Havunen opened this Issue Dec 17, 2015 · 8 comments

Comments

Projects
None yet
4 participants
@Havunen
Contributor

Havunen commented Dec 17, 2015

Hi,

I'm having problem trying to understand how we could use es6 class hierarchy with Riot. Creating and mounting a single tag works ok. (Following this documentation:
http://riotjs.com/api/#riot-tag-impl-conf-innerhtml ), but when another tag needs to be inside this tag Riot doesn't know it exists.

I noticed that for riot to recognize those custom tags in html the tag must be in:

__tagImpl array

However the problem is that to get tags there we must either use riot.tag or riot.tag2 functions, but
these functions want fn as function not as class. Which leads to original issue.
Also its not possible to create tags beforehand, because we cannot give root element which is required parameter for this constructor (riot.Tag).

The best I was able to do was following:

riot.tag2('my-thing', 'html', '', '', function(opts) {
    new MyInheritedThingie(this, opts);
});

But it has the problem that I have to bind everything to "this" because the context is different, which feels wrong.

Has anybody solved this problem already? Or is it just not possible at the moment?

It is related to these discussions: #1273, #1053

Havu

@nippur72

This comment has been minimized.

Show comment
Hide comment
@nippur72

nippur72 Dec 17, 2015

Contributor

Regarding to this context, in RiotTS I have the following trick that works most of the time:

function extend(d, element) {   
      var map = Object.keys(element.prototype).reduce((descriptors, key) => {
         descriptors[key] = Object.getOwnPropertyDescriptor(element.prototype, key);
         return descriptors;
      },{}) as PropertyDescriptorMap;
      Object.defineProperties(d, map);
   }

         var transformFunction = function (opts) {            
            extend(this,classObject);         // copies prototype into "this"                        
            classObject.apply(this, [opts]);  // calls class constructor applying it on "this"
         };

where transformFunction is the function(opts) for riot.tag().

Contributor

nippur72 commented Dec 17, 2015

Regarding to this context, in RiotTS I have the following trick that works most of the time:

function extend(d, element) {   
      var map = Object.keys(element.prototype).reduce((descriptors, key) => {
         descriptors[key] = Object.getOwnPropertyDescriptor(element.prototype, key);
         return descriptors;
      },{}) as PropertyDescriptorMap;
      Object.defineProperties(d, map);
   }

         var transformFunction = function (opts) {            
            extend(this,classObject);         // copies prototype into "this"                        
            classObject.apply(this, [opts]);  // calls class constructor applying it on "this"
         };

where transformFunction is the function(opts) for riot.tag().

@aMarCruz

This comment has been minimized.

Show comment
Hide comment
@aMarCruz

aMarCruz Dec 17, 2015

Member

@Havunen,
riot.tag2 is an internal function used by the compiler, shown in the riot-tmpl docs for debugging purposes, the format of its parameters is subject to change.

Member

aMarCruz commented Dec 17, 2015

@Havunen,
riot.tag2 is an internal function used by the compiler, shown in the riot-tmpl docs for debugging purposes, the format of its parameters is subject to change.

@Havunen

This comment has been minimized.

Show comment
Hide comment
@Havunen

Havunen Dec 18, 2015

Contributor

Thanks for reply guys!
I tried your code snippet @nippur72 , however that gives me error on line

classObject.apply(this, [opts]); // => "Cannot call a class as a function" (babel runtime check)

@aMarCruz yeah we are aware of that, its fine if we can get this working :)

Havu

Contributor

Havunen commented Dec 18, 2015

Thanks for reply guys!
I tried your code snippet @nippur72 , however that gives me error on line

classObject.apply(this, [opts]); // => "Cannot call a class as a function" (babel runtime check)

@aMarCruz yeah we are aware of that, its fine if we can get this working :)

Havu

@nippur72

This comment has been minimized.

Show comment
Hide comment
@nippur72

nippur72 Dec 18, 2015

Contributor

@Havunen as far as I know (see this RiotTS issue), Babel has its own peculiar way of transforming ES6 classes into ES5: it adds a runtime check that you can't turn off (see also this babel closed issue). TypeScript is more friendly in this regard.

Contributor

nippur72 commented Dec 18, 2015

@Havunen as far as I know (see this RiotTS issue), Babel has its own peculiar way of transforming ES6 classes into ES5: it adds a runtime check that you can't turn off (see also this babel closed issue). TypeScript is more friendly in this regard.

@GianlucaGuarini

This comment has been minimized.

Show comment
Hide comment
@GianlucaGuarini

GianlucaGuarini Dec 18, 2015

Member

I think in the next release I will provide a better riot.Tag class that will be able to do the same things you can do with riot.tag.

Member

GianlucaGuarini commented Dec 18, 2015

I think in the next release I will provide a better riot.Tag class that will be able to do the same things you can do with riot.tag.

@Havunen

This comment has been minimized.

Show comment
Hide comment
@Havunen

Havunen Dec 19, 2015

Contributor

I found quite a simple way to make this work but it requires small changes in riot. The main reason it doesn't work as expected is because riot is internally calling new Tag, which always makes new Tag even when there is tag extended from it.

Wrapping new Tag construction into this kind of function would solve the problem, while also keeping backwards support.

function createTag(impl, conf, innerHTML) {
  if (impl.fn && impl.fn.prototype instanceof Tag === true) return new impl.fn(impl, conf, innerHTML)
  return new Tag(impl, conf, innerHTML)
}

This way we also don't need to copy the tag properties, because this context is already correct

EDIT: its not that straightforward, tag options got messed up :(

Then we could give prototype for riot.tag and it should work like before.

For example:

// This class extends riot.Tag and gives base functionality, but its not registered into riot tags
class Human extends riot.Tag {
    constructor(impl, conf, innerHTML) {
        super(impl, conf, innerHTML)
    }
    speak() {
        console.log('Hello!')
    }
}

// Basic pirate functionality
class Pirate extends Human {
    constructor(impl, conf, innerHTML) {
        super(impl, conf, innerHTML);
        this.drunkenness = 0;
    }

    drinkRum() {
        this.drunkenness++;
    }

    speak() {
        if (this.drunkenness === 0) {
            super.speak();
        } else {
            console.log("Yarr!");
        }
    }

    static name() {
        return "Pirate";
    }

    static template() {
        return  '<p>I\'m ' + this.name + '</p><button onclick="{drinkRum}">Drink rum</button><button onclick="{speak}">Speak</button>';
    }
}

// Captain is like pirate, but always drunk and name is different.
// - reusing Pirate tags functionality and layout
class Captain extends Pirate {
    constructor(impl, conf, innerHTML) {
        super(impl, conf, innerHTML);
        this.drunkenness = 100; // Captain is always drunk
    }

    static name() {
        return 'Captain';
    }
}

//Registering tags by prototype
riot.tag2('tag-pirate', Pirate.template(), '', '', Pirate);
riot.tag2('tag-captain', Captain.template(), '', '', Captain);

// Then later somewhere in code we mount app normally ...
// and app tag or any of its child could contain pirate tags
riot.mount('#app', 'tag-app', {});

It renders like this:
renders

Contributor

Havunen commented Dec 19, 2015

I found quite a simple way to make this work but it requires small changes in riot. The main reason it doesn't work as expected is because riot is internally calling new Tag, which always makes new Tag even when there is tag extended from it.

Wrapping new Tag construction into this kind of function would solve the problem, while also keeping backwards support.

function createTag(impl, conf, innerHTML) {
  if (impl.fn && impl.fn.prototype instanceof Tag === true) return new impl.fn(impl, conf, innerHTML)
  return new Tag(impl, conf, innerHTML)
}

This way we also don't need to copy the tag properties, because this context is already correct

EDIT: its not that straightforward, tag options got messed up :(

Then we could give prototype for riot.tag and it should work like before.

For example:

// This class extends riot.Tag and gives base functionality, but its not registered into riot tags
class Human extends riot.Tag {
    constructor(impl, conf, innerHTML) {
        super(impl, conf, innerHTML)
    }
    speak() {
        console.log('Hello!')
    }
}

// Basic pirate functionality
class Pirate extends Human {
    constructor(impl, conf, innerHTML) {
        super(impl, conf, innerHTML);
        this.drunkenness = 0;
    }

    drinkRum() {
        this.drunkenness++;
    }

    speak() {
        if (this.drunkenness === 0) {
            super.speak();
        } else {
            console.log("Yarr!");
        }
    }

    static name() {
        return "Pirate";
    }

    static template() {
        return  '<p>I\'m ' + this.name + '</p><button onclick="{drinkRum}">Drink rum</button><button onclick="{speak}">Speak</button>';
    }
}

// Captain is like pirate, but always drunk and name is different.
// - reusing Pirate tags functionality and layout
class Captain extends Pirate {
    constructor(impl, conf, innerHTML) {
        super(impl, conf, innerHTML);
        this.drunkenness = 100; // Captain is always drunk
    }

    static name() {
        return 'Captain';
    }
}

//Registering tags by prototype
riot.tag2('tag-pirate', Pirate.template(), '', '', Pirate);
riot.tag2('tag-captain', Captain.template(), '', '', Captain);

// Then later somewhere in code we mount app normally ...
// and app tag or any of its child could contain pirate tags
riot.mount('#app', 'tag-app', {});

It renders like this:
renders

@Havunen

This comment has been minimized.

Show comment
Hide comment
@Havunen

Havunen Dec 21, 2015

Contributor

@GianlucaGuarini Is it currently possible to have any kind of OOP hierarchy with tags? We gave up the idea with ES6 classes and tried to implement it with classic prototypes, but that didn't work either. Any ideas?

Contributor

Havunen commented Dec 21, 2015

@GianlucaGuarini Is it currently possible to have any kind of OOP hierarchy with tags? We gave up the idea with ES6 classes and tried to implement it with classic prototypes, but that didn't work either. Any ideas?

@GianlucaGuarini

This comment has been minimized.

Show comment
Hide comment
@GianlucaGuarini

GianlucaGuarini Oct 15, 2016

Member

this has already been solved in riot@3.0.0 #1921

Member

GianlucaGuarini commented Oct 15, 2016

this has already been solved in riot@3.0.0 #1921

@GianlucaGuarini GianlucaGuarini added this to the 3.0.0 milestone Oct 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment