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

stimulus_controller() improperly adds a value attribute when set to false #118

Closed
weaverryan opened this issue Mar 30, 2021 · 1 comment · Fixed by #121
Closed

stimulus_controller() improperly adds a value attribute when set to false #118

weaverryan opened this issue Mar 30, 2021 · 1 comment · Fixed by #121

Comments

@weaverryan
Copy link
Member

Hi!

Suppose you have this controller:

// assets/controllers/testing_controller.js
import { Controller } from 'stimulus';

export default class extends Controller {
    static values = {
        isEnabled: Boolean
    }

    connect() {
        console.log(this.isEnabledValue);
    }
}

Then you enable this in a template:

<div
    {{ stimulus_controller('testing', {
        isEnabled: false
    }) }}
></div>

The expected behavior is that, on load, the log would say false. But, instead, you will get true. The cause is that the HTML is rendering like this:

<div data-controller="testing" data-testing-is-enabled-value></div>

Notice the presence of the attribute (though it's not set to a value). It appears that, for boolean values, (we should triple check this) the presence of the attribute is enough to make it be set to true.

The fix would be to:

When a value is specifically set to false, we would render NO attribute. If it's set to any other value - even null - I think we DO need to set the attribute... because we don't know if the value is a Boolean type from PHP... or maybe a string. Though, I'm not sure if this really matters. But not setting the attribute for null or '' seems... like we might be being too hands on, and the only issue I'm aware of currently is with the boolean.

Cheers!

@jrushlow
Copy link
Collaborator

#121 handles === false - I think maybe we should stick with the strict check for now and perhaps expand to include other falsy values if/when a use case is presented.

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