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

add-use-state: Create useState magic #4417

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

MaquinaTech
Copy link

Hello everyone.
I have created a new magic property to increase security in CSP-compliant developments.
Additionally, you can send variables by parameters to a function, change its value and have it extended.
I hope it helps you and I would love it if we could all improve this new implementation if necessary.

@MaquinaTech MaquinaTech changed the title add-use-state: Creare useState magic add-use-state: Create useState magic Oct 27, 2024
@SimoTod
Copy link
Collaborator

SimoTod commented Oct 27, 2024

Out of curiosity, what's the difference between a state and the already existing Alpine.stores?

I think the example we'll not run with the CSP build of Alpine, not sure if it makes more sense to show a fully functioning example if the aim is to help CSP complaint sites

@MaquinaTech
Copy link
Author

The difference between Alpine.store and $useState is that the store defines the data at a global level and $useState does it at a local level respecting the component hierarchy. The communication between Alpine components is quite poor and define global variables degrades it even more.

@MaquinaTech
Copy link
Author

MaquinaTech commented Oct 27, 2024

To respect CSP, direct insertions into variables in the HTML are not allowed, for example @click="count++"

@ekwoka
Copy link
Contributor

ekwoka commented Oct 28, 2024

I don't see why this should be a core plugin.

the naming is also confusing for this use case.

Does the current CSP build allow...function call expressions?

Checked. It does not.

So this still won't evaluate in expressions anyway.

Instead of this, I'd really just prefer a more advanced expression parser build into the CSP evaluator.

@@ -14,6 +15,7 @@ import './$el'
// Register warnings for people using plugin syntaxes and not loading the plugin itself:
warnMissingPluginMagic('Focus', 'focus', 'focus')
warnMissingPluginMagic('Persist', 'persist', 'persist')
warnMissingPluginMagic('useState()', 'useState', 'use-state')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like you're adding it to the core...

not as a plugin...

so which is it?

Definitely should not be a built in.


test('useState initializes state with the given initial value',
html`
<div x-data="{ state: $useState('testValue') }" x-init="$el.setAttribute('x-data', state)">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All your tests fail


test('useState updates state correctly',
html`
<div x-data="{ state: $useState('initialValue') }" x-init="$el.setAttribute('x-data', state)">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this init even doing?

It will just set x-data = [object Object]...

@SimoTod
Copy link
Collaborator

SimoTod commented Oct 28, 2024

The difference between Alpine.store and $useState is that the store defines the data at a global level and $useState does it at a local level respecting the component hierarchy. The communication between Alpine components is quite poor and define global variables degrades it even more.

Right. In that case, I don't think it solves a big problem in Alpine that would suggest it's needed in the core or even as a first class plugin. x-data has already reactive data scoped locally, the state you define is just a wrapper around a variable.

I had a good look at your PR and I can see why you do it, you are trying to define pure functions that only modify the state and you were not happy because you can't do it for things such as integer literals because they are not references in javascript (like objects are).

<div x-data="{
  count: 0,
  state: {count: 0}
}">
    <button
        @click="increment(count)" // This does not work
        x-text="count"
    ></button>
    <button
        @click="increment2(state)" // This work
        x-text="state.count"
    ></button>
</div>

<script>
    function increment(state) {
        state++;
    }
    function increment2(state) {
        state.count++;
    }
</script>

but in doing so, you are forcing people to use .state and .setState() so it's adds more boilerplate (the majority of the devs will object that it should be transparent like proxy objects to remove DX frictions).

You may not agree with the approach but the Alpine way to do it (or at least one of them) would be to define the logic in your component and not in the global scope so it would look more like

<div x-data="{
  count: 0,
  increment() {this.count++}
}">
    <button
        @click="increment"
        x-text="count"
    ></button>
</div>

or, for those not liking js in the html attributes (CSP build as well), something like

<script>
    document.addEventListener('alpine:init', () => {
        Alpine.data('counter', () => ({
            count: 0,
            increment() {this.count++}
        })
    })
</script>
<div x-data="counter">
    <button
        @click="increment"
        x-text="count"
    ></button>
</div>

That being said, if you like the approach, you can use it in your own project. You can even publish a third party plugin for you to use in all your projects and for other likeminded people who likes the approach but I personally don't feel like it belongs to the core.

@MaquinaTech
Copy link
Author

I find your answer very interesting @SimoTod . You are right in what you say and I will add this tool as an external plugin. I think the most important use of this is to be able to modify the input parameters just like you do in increment2 in a consistent and simple way.

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 this pull request may close these issues.

3 participants