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

Warnings in dev mode showing up for helper functions in version 1.16.0 #492

Closed
ccampbell opened this issue Apr 18, 2017 · 2 comments
Closed
Labels

Comments

@ccampbell
Copy link

ccampbell commented Apr 18, 2017

This seems like a regression in version 1.16.0. I can’t reproduce it on the REPL (since it doesn’t run in dev mode).

As an example I have a component that looks like this

<div id="time" maxwidth="8">{{ timeToDisplayTime(time) }}</div>

<script>
    import { timeToDisplayTime } from '../util';

    export default {
        helpers: {
            timeToDisplayTime
        }
    };
</script>

The generated code looks like this:

function TimeCode ( options ) {
	options = options || {};
	this._state = options.data || {};
	if ( !( 'timeToDisplayTime' in this._state ) ) { console.warn( "Component was created without expected data property 'timeToDisplayTime'" ); }
	if ( !( 'time' in this._state ) ) { console.warn( "Component was created without expected data property 'time'" ); }

timeToDisplayTime should not be checked as part of the state (I don’t think).

This could be an issue with my code. I may not be using the helper functions correctly

@Conduitry
Copy link
Member

The obvious way to address this is to add a !this.helpers.has( name ) condition before adding name to this.expectedProperties. But I'm not sure whether that would be missing some subtler thing. Should the helper be in the dependencies at all? Do we need special handling in the case where someone has an {{#each}} block iterating with a variable that shadows a helper function?

@Rich-Harris
Copy link
Member

I forgot to account for helpers when implementing the new findDependencies method — @Conduitry you're right, helpers shouldn't be treated as dependencies at all.

At the moment we're not considering the possibility that helpers might be shadowed. The code that turns foo into template.helpers.foo happens before local variables are taken into account. That's probably something we could warn about in validation.

(Semi-relatedly, I've wondered if helpers should always take precedence over data etc and not just when they're a CallExpression callee, since at the moment things like array.filter(someFilter) don't work if someFilter is a helper.)

The immediate issue is fixed by #493 though.

Rich-Harris added a commit that referenced this issue Apr 18, 2017
Don't treat helpers as dependencies
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

3 participants