Skip to content

[4.x] Change from dot syntax to named-key access for scoped access in Alpine JS driver#8429

Closed
martyf wants to merge 2 commits intostatamic:4.xfrom
martyf:fix/alpine-js-driver-variable-references
Closed

[4.x] Change from dot syntax to named-key access for scoped access in Alpine JS driver#8429
martyf wants to merge 2 commits intostatamic:4.xfrom
martyf:fix/alpine-js-driver-variable-references

Conversation

@martyf
Copy link
Copy Markdown
Contributor

@martyf martyf commented Jul 11, 2023

The Alpine Data Keys are accessed using a dot-syntax when using scoped data.

If a form's data is:

{
    name: '',
    name_-_dash: ''
}

Then data.name works as expected.

However, data.name_-_dash is not correct.

The issue is that Statamic does allow the dash in a field handle (and in fact, allows very invalid handles to be saved, including spaces, special characters, etc).

While manually removing the invalid character manually works, it is not always that simple to instruct clients to make sure the handle - which is automatically generated for them - doesn't include a dash. While removing the dash is a simple step, it's also an extra human-responsible step that can go wrong.

A more permanent (and less human-responsible) solution should be implemented.

This PR changes the Alpine JS Driver's definition for data access to resolves the issue, and allows dashes in a field handle however only when using scoped data.

This would mean data is accessed using data['name_-_dash"] which is correct.

The benefit of this is that the Statamic CP Blueprint editor can continue working as it currently does but only with scoped data - and the accessing of data is corrected for this use case.

This is only a half fix though. If you do not use scoped data, you're out of luck.

There are two more permanent solutions:

  1. Update the Blueprint builder for Forms only to not allow dashes (or any other JS-invalid characters - currently there's no validation) in handles (both with the Slugify generation, and validation of the slug), or
  2. Change the policy to always used scoped data with the Alpine driver

Both of these bigger than just a simple fix and are a bit opinionated, but happy to chat further about how we've seen the Alpine JS driver used and work towards a more permanent fix.

Given this is only a half-fix, I appreciate it may not be accepted... but also is a bandaid fix to the issue too especially if using more complex forms and Alpine setups.

@martyf martyf changed the title Change from dot syntax to named-key access for scoped access in Alpine JS driver [4.x] Change from dot syntax to named-key access for scoped access in Alpine JS driver Jul 11, 2023
@jasonvarga
Copy link
Copy Markdown
Member

I'm happy for a band-aid, but we should really prevent people from using dashes in field handles.

@jasonvarga jasonvarga requested a review from jesseleite July 11, 2023 14:11
@martyf
Copy link
Copy Markdown
Contributor Author

martyf commented Jul 11, 2023

I feel this is a slippery slope of an issue as it impacts a few areas:

For the frontend:

  • realtime updates/validation of the handle
  • checking the handle after an update (i.e. after a manual change)
  • potential differing rules for handles for different object types

For the backend:

  • validating the handle only has the allowed characters

And for a version update:

  • what happens to invalid handles during an upgrade?

That last one is a little scary as it has potential to impact what users have already done (maybe they've written their own JS that relies on the handle - but if the upgrade changes the handle then that would break their JS).

If nothing happens during the upgrade (i.e. nothing gets migrated) then the user needs to manually make changes which could be big (or small, or not an issue at all) - but also if they're not having issues with invalid characters, it's almost like they should be able to save without re-writing handles (what if they're fixing a typo elsewhere in the field/blueprint?).

The speakingurl could be configured to not allow a dash (so that dashes in slugs are fine, but not Form blueprints):

{
    separator: "_", 
    custom: { '-' : '' } 
}

This would take Name - Dash and return name_dash. Basically had a quick play with speakingurl to see what would happen - but does require this odd custom dictionary that could also contain any other characters that need to be excluded from the handles.

Happy to try to help out where I can - but looking deeper is bigger than just a simple change as I think it has an impact across the whole platform.

@jesseleite
Copy link
Copy Markdown
Contributor

Hey Marty! Thanks for the PR and all the thought you put into this ❤️

One thing I found though is that this fix only works with scoped alpine data (as you said). This still errors when using js="alpine" without scoping, because dashes cannot be used in JS variable names (ie. x-model="some_-_var", where that dash is actually interpretted as a - minus operator in JS).

I think the first step is to prevent dashes in field handles as Jason pointed out. We agree that some thought definitely needs to be put into the best way to tackle this for existing apps, but it's something that needs to be tackled at that level anyway, as dashes have caused problems in the new antlers parser, etc. as well.

Closing this for now, but it's on our list for after Laracon 👍

@jesseleite jesseleite closed this Jul 14, 2023
duncanmcclean added a commit that referenced this pull request Feb 1, 2024
I've added test cases for some of the slug issues we've fixed in the past, including:

* #8429
* #8749
* #8844
* #5823
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