search

silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
723 stars 821 forks source link

ENH Convert scalars and arrays to viewable data #11227

Closed emteknetnz closed 4 months ago

emteknetnz commented 4 months ago

Issue https://github.com/silverstripe/silverstripe-framework/issues/11196

GuySartorelli commented 4 months ago

I had to dive in and try to get this working myself to wrap my head around it. I ended up with these commits which handle allowing primitives to be used in templates, and allowing iterators such as ArrayIterator to work natively in templates.

I cam to the same conclusion as you regarding arrays though - even though SSViewer_Scope::next() has a comment saying the item could be an array, and handles that, there's no way to actually have an array there unless we update ViewableData::obj() to return them.

What should we do?

I think we need to do two things:

  1. In CMS 5 allow primitives other than array. This is an improvement for simple use cases and doesn't require any BC breaking changes as my attempt shows. Looking at the commit history in this PR you had something very similar originally as well.
  2. In CMS 6 make whatever changes are necessary to get arrays working natively in templates. This could mean:
    1. Update ViewableData::obj() to return arrays directly if there are any
    2. Update ViewableData::obj() to wrap arrays in either a list or ArrayData depending on what type of array they are
    3. Update ArrayList to either never wrap arrays, or to always wrap them in either a list or ArrayData depending on what type of array they are
    4. Some combination of the above
    5. Decoupling the logic from ViewableData - though that may be more effort than it's worth
    6. Something else neither of us considered yet, if someone else has any ideas.

I'd say update this PR to handle point 1 above, and raise a new card to look at point 2 since that'll need to go through refinement.

Considering there's a ton of ways point 2 could go, that may even be worth discussing in an architecture catchup.

emteknetnz commented 4 months ago

I'll close the PR to retain the diff for the CMS 6 card and open a new one to do just the scalars