Methods corresponding to scaffolded fields can no longer return plain arrays, error message cryptic

[MasonD]

It used to be such that the following would work:

ClassName:
  fields:
    customField:
      type: '[String!]!'
    property: getCustomField

class ClassName extends DataObject {
  // ...
  public function getCustomField() { return ['custom', 'field']; }
}

But now this causes errors reading nl2br(): Argument #1 ($string) must be of type string, array given, which is rather cryptic.

The cause is that, now that scaffolded fields are accessed through ViewableData::obj(), the array goes through the casting process and by default will be cast as a DBString, which will use nl2br when forTemplate is called. There doesn't seem to be a way to achieve the desired result besides instead returning an ArrayList.

(Creating an ArrayList for me seems like not the optimal solution, because it might necessitate creating an extra function specifically for graphql if there's oither code already expecting getCustomField to return a plain array)

I don't know if this is something to fix in some way (however that might be), but if not I think there need to be warnings in the documentation somewhere, or the error caught and given a better error message, because right now somebody not familiar with the internals of silverstripe-graphql will have no idea how to proceed when encountering this error.

[emteknetnz]

@MasonD which version of graphql are you using? 3 or 4 (master branch)?

[MasonD]

Version 4.

[emteknetnz]

@unclecheese

[unclecheese]

Yup, we need to coerce array results into ArrayList.. that's the best option here. I'm thinking....

Extension for all dataobjects provides graphqlObj(), which:

• hasMethod($field) ?
  • No? go to -> obj($field)
  • Yes?
    • Test the return value.. either execute the method, or
    • Rely on return type definition (e.g. reflection)
    • If array:
      • Wrap it in ArrayList
    • Else -> obj($field)

Then, update all the ->obj references throughout (mostly FieldAccessor) to use graphqlObj

[MasonD]

That seems like a lot of extra introspection to be done on every resolution of a field, if that logic lives in the resolver. Perhaps it can be done at compile time and the resolver can be set as either one that calls obj or one that calls the method and wraps in ArrayList? Or is that premature optimisation...

[unclecheese]

Nah, this would all happen in the build. You can pass metadata to the resolver at compile time with addResolverContext, e.g. addResolverContext(['isMethodThatReturnsArray', => true])

[bummzack]

I've got the same issue when I try to return an array of strings. Wrapping the result in an ArrayList does not help either.
How did you end up solving this issue, @MasonD?

When returning an ArrayList of strings, I now get:

[Emergency] Uncaught TypeError: get_class(): Argument #1 ($object) must be of type object, string given