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

[CSP] Add support for nested properties to CSP build #4238

Merged
merged 3 commits into from
May 28, 2024

Conversation

SimoTod
Copy link
Collaborator

@SimoTod SimoTod commented May 25, 2024

This PR adds a minimal enhancement to the CSP build to support nested properties via dot notation (property accessor).

<div x-data="counter">
    <button @click="foo.increment">Increment</button>
    <span x-text="foo.count"></span>
</div>
Alpine.data('counter', () => ({
    foo: {
        count: 1,

        increment() {
            this.count++
        },
    },
}))

The build size is pretty much the same and it seems a reasonable change otherwise dev using the csp build are forced to flatten the component and put all properties at the top.

I only know about one user mentioning it though so feel free to close it if you think it's not worth it for now.

@SimoTod SimoTod changed the title Add support for nested properties to CSP build [CSP] Add support for nested properties to CSP build May 26, 2024
runIfTypeOfFunction(receiver, completeScope[expression], completeScope, params)
let evaluatedExpression = expression.split('.').reduce(
(currentScope, currentExpression) => {
if (currentScope[currentExpression] === undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (currentScope[currentExpression] === undefined) {
if (!currentExpression in currentScope) {

should help in scenarios where the thing is expected to be undefined.

But also, this will now exhibit a different behavior than non-csp in regards to this, where it would only error when trying to access a key on the thing that is undefined, but would be happy to return undefined.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point, I kept the same check as the original version. Happy to change it if Caleb is cool with that.

@calebporzio calebporzio merged commit 2ddfff2 into alpinejs:main May 28, 2024
1 check passed
@calebporzio
Copy link
Collaborator

This is great. Thanks Simone!

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