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

String variables not correctly resolved #49

Closed
sebastiansandqvist opened this issue Aug 2, 2016 · 4 comments
Closed

String variables not correctly resolved #49

sebastiansandqvist opened this issue Aug 2, 2016 · 4 comments
Labels

Comments

@sebastiansandqvist
Copy link

I have the following component which works without mopt enabled, but is broken with mopt:

const NEWEST = 'Newest first';
const VOTES = 'Most votes';
const SortToggle = {
    view({ attrs }) {
        return (
            m('.SortToggle',
                m('.SortToggle-option', { onclick() { attrs.sortBy(NEWEST); } }, NEWEST),
                m('.SortToggle-option', { onclick() { attrs.sortBy(MOST_VOTES); } }, MOST_VOTES)
            )
        );

    }
};

It works fine using mopt if I inline the string constants, like this:

const NEWEST = 'Newest first';
const VOTES = 'Most votes';
const SortToggle = {
    view({ attrs }) {
        return (
            m('.SortToggle',
                m('.SortToggle-option', { onclick() { attrs.sortBy(NEWEST); } }, 'Newest first'),
                m('.SortToggle-option', { onclick() { attrs.sortBy(MOST_VOTES); } }, 'Most votes')
            )
        );

    }
};
@tivac tivac added the bug label Aug 2, 2016
@tivac
Copy link
Contributor

tivac commented Aug 2, 2016

Thanks for the report!

Can you provide examples of the broken output on a minimal repro? That'd be a huge help for figuring out the issue here!

@sebastiansandqvist
Copy link
Author

Sure, here you go: https://github.com/sebastiansandqvist/mopt-bug-repro

@sebastiansandqvist
Copy link
Author

The issue appears to only happen if both of the following conditions are met:

  • a variable is being used rather than a literal as the text node
  • the component contains a second argument for attrs

@tivac
Copy link
Contributor

tivac commented Aug 2, 2016

mithril-objectify isn't compatible with the rewrite branch of mithril.

See MithrilJS/mithril.js#1086 for why. There is a rewrite branch where I was working on support but I've held off on further work because there still isn't a great way to support the underlying vnode structure from a static analysis tool.

@tivac tivac closed this as completed Aug 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants