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

Casing of signals/computed signal names #450

Closed
khalwat opened this issue Jan 4, 2025 · 5 comments · Fixed by #451
Closed

Casing of signals/computed signal names #450

khalwat opened this issue Jan 4, 2025 · 5 comments · Fixed by #451

Comments

@khalwat
Copy link
Contributor

khalwat commented Jan 4, 2025

It appears that signal names and computed signal names defined like data-signals-gradientName & data-computed-gradientName are lower-cased before the computed signals are created.

This works:

         data-computed-gradientcolor="$pokemonType"
    >
        <h1 data-text="$gradientcolor"></h1>

but this doesn't work:

         data-computed-gradientColor="$pokemonType"
    >
        <h1 data-text="$gradientColor"></h1>

so it looks like it's lower-casing what I name the attribute, so this works:

         data-computed-gradientColor="$pokemonType"
    >
        <h1 data-text="$gradientcolor"></h1>

which is frankly a little weird (and undocumented as far as I can tell).

I think signal and computed signal name case should be honored, so camelCase (or whatever people want) can be used for signal names. CamelCase and other casing does work if signals are declared via object syntax, e.g.:

<div data-signals="{ gradientColor: 'green' }"</div>
<h1 data-text="$gradientColor"></h1>

...which is inconsistent, and also, of course, we can't use any kind of object syntax for defining computed signals.

If this is due to limitations of the HTML spec, then probably:

  1. It should be prominently noted whatever the naming restrictions are
  2. Signal and computed signal names should be normalized to be lowercase everywhere

That means that $gradientColor would be normalized to $gradientcolor before it is evaluated in an expression, and object keys in data-signals="{}" should be normalized to lowercase as well, just like they are when I do data-signals-gradientName

@khalwat
Copy link
Contributor Author

khalwat commented Jan 4, 2025

Just discovered that I can get my camelCase this way (which makes my normalizing comments above somewhat moot):

         data-signals-gradient-color="$pokemonType"
    >
        <h1 data-text="$gradientColor"></h1>

Definitely needs to be documented somewhere. Yes, I realize this is just how data-* attributes work when accessing them via JavaScript, but given how Datastar leverages them, it's unclear how it is going to convert them into signal names.

I think it should be noted in the Datastar documentation that it will do this automatic conversion from kebab-case to camelCase for data- attribute names.

So let's call this a documentation issue.

@khalwat
Copy link
Contributor Author

khalwat commented Jan 4, 2025

Interestingly, this little trick is supposed to work, but doesn't:

         data-signals--gradient-color="$pokemonType"
    >
        <h1 data-text="ctx.signals.JSON()"></h1>

Outputs:

{"-GradientColor": "Electric" }

When it should be:

{"GradientColor": "Electric" }

...according to this.

@bencroker
Copy link
Collaborator

Agreed that some of this should be documented. Raised a PR for the key conversion issue at #451.

@khalwat khalwat closed this as completed Jan 4, 2025
@bencroker
Copy link
Collaborator

bencroker commented Jan 4, 2025

The docs currently state:

Note that data-* attributes are case-insensitive. If you want to use uppercase characters in signal names, you’ll need to kebabize them or use the object syntax. So the signal name mySignal must be written as data-signals-my-signal or data-signals="{mySignal: 1}".

If this is insufficient, please make a suggestion.

@bencroker
Copy link
Collaborator

Released the fix in v1.0.0-beta.2.

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

Successfully merging a pull request may close this issue.

2 participants