Skip to content
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

Proposal: Marko Widgets with ES6 and inline Marko templates #152

Open
patrick-steele-idem opened this issue Aug 2, 2016 · 6 comments
Open

Comments

@patrick-steele-idem
Copy link
Contributor

patrick-steele-idem commented Aug 2, 2016

We are exploring how we can improve Marko Widgets by leveraging the ES6 syntax and here are the initial thoughts:

import { Component } from 'marko-widgets';

export default class extends Component {

    render(state, input) {
        var fullName = state.firstName + ' ' + state.lastName;

        return marko`<div> <!-- w-bind is implicit -->
            <app-hello w-id="hello" w-onclick='handleButtonClick' name=fullName />
        </div>`;
    }

    constructor() {
        // This is called on the server and in the browser, but you
        // probably don't want to do anything here

        // *Don't* call super: super();

        // this.el === undefined
    }

    onInput(input) {
        this.state = {
            firstName: this.state.firstName || input.firstName,
            lastName: input.lastName
        };

        // Any additional properties will be serialized to the browser if rendered on the server
        this.foo = { hello:'world' };
    }

    onMount() { // This is a new lifecycle event method
        // this.el is available here, but not in the constructor
        var el = this.el;

        this.helloWidget = this.getWidget('hello');
    }

    handleButtonClick(event, el) {
        this.doSomething(); // This is okay
    }

    doSomething() {

    }
};

Feedback is appreciated

@patrick-steele-idem patrick-steele-idem changed the title Proposal: Marko Widgets and inline Marko templates Proposal: Marko Widget with ES6 and inline Marko templates Aug 2, 2016
@patrick-steele-idem patrick-steele-idem changed the title Proposal: Marko Widget with ES6 and inline Marko templates Proposal: Marko Widgets with ES6 and inline Marko templates Aug 2, 2016
@abiyasa
Copy link

abiyasa commented Aug 16, 2016

Looks great! 👍 👍 👍

I really like the compact code. Combined with inline template, we could have a Marko Widget component in just 1 file. Well, we still have marko-tag.json file, but maybe we could combine that into the class using 'static' property.

export default class CustomComponent extends Component {

    render(state, input) {
        var fullName = state.firstName + '
...
}

CustomComponent.tag = {
    attributes: {
        model: 'expression'
    }
};

What do you think? Some other questions:

  • Is the new onMount() replacing the old init()? That's cool & reminds me of React's componentDidMount
  • How can we implement split renderer & widget in ES6?

@patrick-steele-idem
Copy link
Contributor Author

Well, we still have marko-tag.json file, but maybe we could combine that into the class using 'static' property.

For the most part, marko-tag.json is optional, but it is used compile-time validation and it is also used by third-party tools. I think it is best to keep the marko-tag.json in a separate JSON file to avoid the overhead of having to parse the entire JS file just to find the tag def since that would slow down autocomplete and compilation.

Is the new onMount() replacing the old init()? That's cool & reminds me of React's componentDidMount

Yes, onMount() would replace init(). Having similarity with other view libraries is a good thing and to deviate where it makes sense for performance reasons and usability (e.g. cleaner Marko syntax).

How can we implement split renderer & widget in ES6?

Good question. We do plan on keeping complete backwards compatibility so you could always use the old syntax, but it probably makes sense to offer extends Renderer and extends Widget as an alternative to extends Component.

Thanks for the feedback @abiyasa!

@abiyasa
Copy link

abiyasa commented Aug 16, 2016

Thanks for the reply. I'm not aware of that use case for marko-tag.json. Make more sense to keep it as separated file then.

We do plan on keeping complete backwards compatibility so you could always use the old syntax, but it probably makes sense to offer extends Renderer and extends Widget as an alternative to extends Component.

Extending class from Renderer or Widget classes sounds good!

@basickarl
Copy link

basickarl commented Oct 25, 2016

Sneaking in here and saying that the syntax looks good. As @abiyasa said would be nice to somehow avoid writing import { Component } from 'marko-widgets';, on the other hand it would be needed to define extends Renderer and extends Widget (which is a good idea).

I was looking at reactjs syntax and they skip the export default, any reason for keeping it here (thinking as you said earlier, would be good to get the syntax closer to each other)?

Again looking at reactjs they state React components can have state by setting this.state in the constructor. I'm guessing in your example that is the same as onInput? I'm guessing it's not in the constructor as onInput can be invoked several times (change state at runtime)?

Woudl also be nice to get rid of the exports.render = require('./renderer').render; in the index.js when splitting up the Component into the Wdiget and Renderer'.

Apart from that looks awesome, hows it going? :P

@patrick-steele-idem
Copy link
Contributor Author

Thanks for the input, everyone. @mlrawlings had started the work on this, but after some discussion we realized we can simplify things even more. We still want to include support for onMount and onInput, but with the new proposal we see no benefit from supporting ES6. I will write up a new proposal based on our discussions for feedback and link here once done. I'm confident that everyone will like the simplifications with the new proposal :)

@patrick-steele-idem
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants