ZCLI's zcli.config.apps.json parameters doesn't stringifies objects like ZAT did

joelhellman commented 1 year ago

Expectations

I have app parameter setting of type multiline in manifest.js that holds some app settings stored locally as nested JSON.

Now, ZCLI removed settings.json and introduced parameters in zcli.config.apps.json.

When I had this kind of manifest file in ZAT

"parameters": [{"name": "jsonfield", "type": "multiline"}]

and this kind of settings.json file in ZAT: {"jsonfield":{"meaningOfLife": 42}}

and I ran this code:

const client = ZAFClient.init();
client.on('app.registered', function (appRegisteredData) {
    const { metadata } = appRegisteredData;
    const jsonfieldType = typeof metadata.settings.jsonfield;
    console.log(jsonfieldType);
})

the console.log returned "string"

ie. the zat server maybe mapped "jsonfield" in the settings.json to the manifest's multiline field, noticed it was a string (multiline is text) and stringified the value. If one logs the metadata.settings objects, I get

?zat=true

Btw, this way of loading settings from app.registered seems okay to me, I've used it in many apps, it's also officially endorsed in docs: https://developer.zendesk.com/documentation/apps/build-an-app/using-react-in-a-support-app/

To the issue

I expected this zcli.config.apps.json content: {"parameters": {"jsonfield": {"meaningOfLife": 42}}} run by zcli apps:server:

const client = ZAFClient.init();
client.on('app.registered', function (appRegisteredData) {
    const { metadata } = appRegisteredData;
    const jsonfieldType = typeof metadata.settings.jsonfield;
    console.log(jsonfieldType);
})

to also console log "string"

Reality

If I have zcli.config.apps.json content: {"parameters": {"jsonfield": {"meaningOfLife": 42}}} and run zcli apps:server it doesn't returns string but 'object'

Logging the metadata.settings object, I can see the whole parameter JSON root has now already been parsed as a full document, instead of applying the business logic that fields cannot store native JSON in Zendesk v2 apps (AFAIK).

?zcli_apps=true

ZAT knew that app settings fields couldn't store JSON, so it stringified the JSON if it noticed an object. ZCLI doesn't account for this.

Another thing is that issue #14 introduced that parameters in zcli.config.apps.json are auto-sent into to the app installation settings when using apps:update. However that will also send the parameter as an object, which Zendesk's ruby backend will then try to serialize.

In the my example, the apps installation setting - which was sent zcli apps:update from parameters in zcli.config.apps.json without any stringification - now landed like this in the production installation app setting as:

"settings": {
   ...
   "jsonfield": "{\"meaningOfLife\"=>42}",
},

and for me, this caused the deployment step of zcli apps:update to stall indefinetly at the deploy step (w/o any error message or timeout)

$ zcli apps:update dist
Uploading app... Uploaded
Deploying app... ⣯    <--- spinned forever

My workaround

So now for my code not to break when I use parameters in zcli.config.apps.json, I had to change my initialization to something like this

const client = ZAFClient.init();
client.on('app.registered', function (appRegisteredData) {
    const { metadata } = appRegisteredData;
    const jsonfieldType = typeof metadata.settings.jsonfield;
    let jsonFieldTypeObject;
    if (jsonfieldType === 'string') {
       try {
            jsonFieldTypeObject  = JSON.parse(metadata.settings.jsonfield);
       } catch (e) {
         // handle error
       }
    } else {
       jsonFieldTypeObject  = metadata.settings.jsonfield;
    } 
})

Which complicates things for me because the app settings now behaves differently once deployed from when I test locally.

Another approach would be to not accept nested JSON in parameters in zcli.apps.config.json. I.e. require each parameter settings there to already be a stringified JSON. I feel that would result in a bad local DX, because it's hard to write stringified JSON by hand.

Fix

Maybe this is too late to fix, since it might mess with existing ZCLI apps, but otherwise I would like if we could have back the original behavior from ZAT, that the zat server considered that app installations cannot have nested objects, so they are JSON stringified by zcli apps:server when running locally with parameters in zcli.config.apps.json the file.

(For the payload sent on update in zcli apps:update in #14, that behavior should follow, altough the current behavior there seems weird to me, as I reported in #183)

Steps to Reproduce

clone https://github.com/joelhellman/zendesk-zat-vs-zcli-settings-repro
zcli apps:server
check output with zcli_apps=true - it will be "object", check dev console
zat server -c settings.json
check output with zat=true - it will be "string", check dev console

Issue details

Command: zat apps:server
Version: @zendesk/zcli/1.0.0-beta.33 wsl-x64 node-v18.13.0
OS: Unbuntu22/WSL2

zach-anthony commented 1 year ago

Hi Joel, thanks for the detailed description of the issue. I agree that the typeof operator should not be returning Object when testing your app with ZCLI as this would not be possible, once the app has been installed in your instance. Having reviewed usage from other developers, it appears that the pattern of using nested objects or arrays in 'multiline' parameters is quite common.

While I think that this signals that we ought to have a more suitable types for parameters, I think it makes sense for us to support the same behaviour as ZAT in terms of stringifying objects and arrays (perhaps with a warning or info statement that lets the developer know this will happen)

I can't commit to when we'll be able to release this feature, but have added this to our internal backlog and will keep this issue open until resolved

schiguoi commented 9 months ago

+1 on fixing this, I spent far too long trying to figure out why this worked differently from zcli.apps.config.json and post-install using the same settings. Thanks @joelhellman for posting your solution, it was helpful in working around this.

zendesk / zcli