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

[5.x] Adjust behavior of array fields #10467

Merged
merged 27 commits into from
Aug 9, 2024
Merged

Conversation

duncanmcclean
Copy link
Member

@duncanmcclean duncanmcclean commented Jul 17, 2024

This pull request makes some changes to how Array fields are stored, to fix some long-standing issues.

Currently, Array fields are stored like this, in a simple key/value array:

foo: Foo
bar: Bar
baz: Baz

While this format works well most of the time, everything falls apart when you try to use integers or floats as keys due to how arrays work in YAML. 🫠

To workaround this, we've decided to inflate the key/values like this:

-
  key: foo
  value: Foo
-
  key: bar
  value: Bar
-
  key: baz
  value: Baz

The Array Fieldtype will continue being able to read from the legacy format, then, when you save the array, it'll convert the field to the new format.

Fixes #3179.
Fixes #2215.
Fixes #5969.

duncanmcclean and others added 6 commits July 17, 2024 16:02
Returning the options from `meta` means we can transform the data into a "common" format in PHP-land, before using it on the Vue side.
Copy link
Member

@jasonvarga jasonvarga left a comment

Choose a reason for hiding this comment

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

This unfortunately all breaks down when you want to use an array field in your content.

You can no longer do {{ array_field:keyname }} in your templates anymore.

I think maybe we only save the expanded syntax when the fieldtype is being used for config, if possible.

- for pre/process and augment for keyed and dynamic versions
- multi-dimensional should only get saved when we're dealing with numeric keys since thats when yaml borks
- keep old associative array syntax where possible for backwards compatibility
@ryanmitchell
Copy link
Contributor

With the changes will this still close statamic/eloquent-driver#312 ? I was hoping this PR would consistently store everything as key/value but this doesn't seem to be the case any more?

@jasonvarga
Copy link
Member

Bummer. I was trying to avoid changing data where possible. It's sort of a breaking change.

If people were doing something like $entry->get('array_field') and expecting it to be ['foo' => 'bar', 'baz' => 'qux'] it will now be [['key' => 'foo', 'value' => 'bar'], ['key' => 'baz', 'value' => 'qux']] if they resave the entry.

I can make the augmented version output the old way, that part is fine though. e.g. in templates {{array_field:foo}} would still work.

@ryanmitchell
Copy link
Contributor

ryanmitchell commented Aug 6, 2024

`I can make the augmented version output the old way, that part is fine though. e.g. in templates {{array_field:foo}} would still work.

I was selfishly hoping this would be enough but you're right its breaking so shouldnt be done at this point in the cycle.

I'll get back to work on the eloquent PR.

@jasonvarga
Copy link
Member

Maybe we make the new format opt-in per field. Then opt in each config field.

In v6 we could remove the option and make it the only behavior.

@jasonvarga
Copy link
Member

By the way, it's crazy to me that object key order can't be maintained. It looks like it's maintained in sqlite but not in mysql.

@ryanmitchell
Copy link
Contributor

By the way, it's crazy to me that object key order can't be maintained. It looks like it's maintained in sqlite but not in mysql.

Totally, its absolute nonsense!

@jasonvarga
Copy link
Member

@ryanmitchell This fixes your issue. Reordering options of a select field will use the order you defined, in MySQL. 👍

@ryanmitchell
Copy link
Contributor

Thank you

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