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

Expose value attribute control via schema. #683

Open
panoply opened this issue Jun 7, 2023 · 9 comments
Open

Expose value attribute control via schema. #683

panoply opened this issue Jun 7, 2023 · 9 comments

Comments

@panoply
Copy link

panoply commented Jun 7, 2023

Stimulus schema definitions are rather extraneous out of the box and though schema can be controlled in the Stimulus runtime, the current customisations only provide control over the following:

export interface Schema {
    controllerAttribute: string;
    actionAttribute: string;
    targetAttribute: string;
    targetAttributeForScope(identifier: string): string;
    outletAttributeForScope(identifier: string, outlet: string): string;
    keyMappings: {
        [key: string]: string;
    };
}

I'd like to have control over values defined on controllers, Take the following code snippet I've pulled from a project:

  <div
    data-controller="locale"
    data-locale-country-value="Netherlands"
    data-locale-language-value="Dutch"
    data-locale-market-id-value="12345678"
    data-locale-return-price-value="105"
    data-locale-return-threshold-value="200"
    data-locale-root-url-value="/en-nl"
    data-locale-shipping-price-value="12"
    data-locale-shipping-threshold-value="200">
</div>

This is the stimulus default and while it suffices for small cases when a controller requires a lot of runtime state things can become difficult to wrangle on the readability level. I'd like to propose or alternatively be able to apply the following schema structure for defining controller state:

(using the above sample):

  <div
    controller="locale"
    locale:country="Netherlands"
    locale:language="Dutch"
    locale:market-id="12345678"
    locale:return-price="105"
    locale:return-threshold="200"
    locale:root-url="/en-nl"
    locale:shipping-price="12"
    locale:shipping-threshold="200">
</div>

Reasoning

In Stimulus, state can only be passed on controller annotated elements and this gives us an isolation benefit with DOM defined state. Unless the internal handling for controller values are dependending upon the -value suffix in detrimental manner, is there any real necessity for it? I get that it signals "this X value used by the Y controller" but is this not already inferred given that controller specific elements are the only point where DOM state definitions can be passed?

In terms of the data- prefixes, unless stimulus is leveraging dataset methods, similar to the -value suffix, are they really necessary? The legacy argument could be made here but Stimulus and Turbo both rely upon modern web capabilities and as such, custom attribute expressions can be leveraged (such as colon : separators which can be queried by escaping).

Curious on the thoughts of other developers leveraging Stimulus.

Thanks.

@marcoroth
Copy link
Member

Hey @panoply, thanks for opening this issue!

I'm wondering why we currently don't have the possibility to configure the Schema/attributes for the Values API. I think it would make sense to also provide this functionality so that people can customize the Schema to their liking.

For consistency reasons I think it still makes sense to use the data-[identifier]-[value]-value notation by default. But with the possibility to override the Schema you could do something like the notation you proposed:

// src/application.js
import { Application, defaultSchema } from "@hotwired/stimulus"

const schema = {
  ...defaultSchema,
  valueAttributeForScope: (identifier, value) => `${identitifer}:${value}`
}

Application.start(document.documentElement, schema);

In this case you just need to be careful that you don't clash attributes, because locale:country="Netherlands" wouldn't indicate if this is a Value, Target, Class or Outlet. Just something to keep in mind.

@rik
Copy link
Contributor

rik commented Jun 15, 2023

Another thing to keep in mind, locale:country is not a valid custom data attribute name per HTML.

@marcoroth
Copy link
Member

@rik yeah right. But apart from that I think it's still valuable to have the data attributes for the Values API configurable in the Schema.

Ultimately, it's up the developers then if they decide to use non-valid HTML attributes for their application.

There are also a few other JavaScript frameworks out there which use a similar notation to the one proposed in this issue, even if it's not valid HTML. I guess this is where the desire for such a notation is coming from. Personally I don't see a reason why we shouldn't allow this.

@panoply
Copy link
Author

panoply commented Jun 15, 2023

Another thing to keep in mind, locale:country is not a valid custom data attribute name per HTML.

There is nothing stopping anyone from using attributes of that structure. We are talking about Markup here. Colon separators are leveraged within a multitude of frameworks (Alpine, Vue and others). One can also traverse, query and reason about with such attributes by simply escaping, for example:

const element = document.querySelector('[locale\\:country]');
const attribute = document.body.firstElementChild.getAttribute('locale:country'):

Both will suffice.

See Flems

@rik
Copy link
Contributor

rik commented Jun 15, 2023

@rik yeah right. But apart from that I think it's still valuable to have the data attributes for the Values API configurable in the Schema.

Oh yeah, I think the feature is valuable.

I'd love it if the framework added a bit of friction to discourage using invalid custom data attribute names but that's probably too late to do so given the existing schema customisations.

There is nothing stopping anyone from using attributes of that structure.

I know it's a common practice in the industry but HTML validators will declare such markup invalid.

@panoply
Copy link
Author

panoply commented Jun 15, 2023

I know it's a common practice in the industry but HTML validators will declare such markup invalid.

I hear you. I understand from a specification point where you are coming from and adhering to the standards is better than to bend and augment around that.

I'd be happy to do some ground here if required. Adhering to the current notation is fine but to echo what @marcoroth was saying, it would be nice to have that consistency for custom schema given values are the only missing extensibility point.

@marcoroth
Copy link
Member

@panoply do you want to give a shot?

@panoply
Copy link
Author

panoply commented Jun 18, 2023

@marcoroth yeah, no problems. I will sort something this week.

@marcoroth
Copy link
Member

@panoply sweet, thank you! Let me know if you need any assistance 🙌🏼

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

No branches or pull requests

3 participants