Lazy state: doesn't work in non-operation function arguments

[josephjclark]

This:

fnIf(
  Object.values($.results).length === 0
  state => console.log(`Couldn't find records for '*:${state.cursor}*'`)
}

Compiles to this:

fnIf(Object.values(state => state.results).length === 0, state =>
  console.log(`Couldn't find records for '*:${state.cursor}*'`)
)];

Which is wrong. We needed to walk a bit higher up the try to work out where to put the wrapping function.

Should be:

fnIf(
  state => Object.values(state.results).length === 0
  state => console.log(`Couldn't find records for '*:${state.cursor}*'`)
}

[doc-han]

@josephjclark @mtuchi

Is it correct to say that all exposed adaptor functions have arguments that can be callbacks which get called with state?
I mean all arguments go through some form of expandReferences.

[mtuchi]

@doc-han not all adaptor functions uses expandReferences.
There are couple adaptor functions that are still not using expandaReferences. And also util functions (eg util.encode) are not using expandaReferences

[doc-han]

Alright, thanks @mtuchi
So these utilities are to be used only under the util namespace right? if so, that's fine

Apart from them, and callbacks that are actually state callback like the one from get.
do we still have adaptor functions that don't use expandReferences?

I've been through most adaptors and so far they seem to use expandReferences. will be glad knowing the ones that don't(happy if not many).

[josephjclark]

Notes from our call to resolve this issue:

1. Only consider $ to be lazy state if it is NOT inside a new scope (arrow function / function block). Ie, ignore lazy state in callbacks
2. When replacing arrow functions:3a. IF NOT AN OBJECT PROPERTY always walk to the top-level "argument root" (not just the parent). This solves the issue3b. IF AN OBJECT PROPERTY replace the "value root"

I don't want to do import tracking (yet) because it's still too hard to know which imports are operations

We will not relax the state keyword requirement, becuase we will never try to replace a $ operator inside nested code (therefore the user never has control over the state name`

So we don't support this:

create(newState => {
   name: $.data.name  // ILLEGAL!!!!
})

Some notes about nested state

// This works great today
create({
  name: $.joe,
  country: $.uk
  fullName: `${$.data.fname} ${$.data.fname}`,
})

// we'd compile it like this
create({
  name: (state) => state.joe,
  country: (state) => state.uk
  fullName: (state) => `${state.data.fname} ${state`.data.fname}`,
})

// But if we walk to the argument root, we do this:
create(state => ({
  name: state.joe,
  country: state.uk
  fullName: `${state.data.fname} ${state.data.fname}`,
}))

// Which is sort of fine, but we break this use case:
create({
  name: $.joe,
  country: (state) => {
    const targets = state.items.map(() => { .. })
    targets.find(...)
    return targets[0]
  },
  fullName: `${$.data.fname} ${$.data.fname}`,
})

[doc-han]

@josephjclark there's a problem with the example I provided during the call

Currently

#	Source	Compiles to	status
1	each($.users, post($.name))	each(state=> state.users, post(state => state.name))	Works ✅
2	fnIf(Object.values($.results))	fnIf(Object.values(state => state.results))	Problematic ⛔️

Our current ticket is around fixing 2 and we agreed walking to the top call-expression will do the trick but then, that breaks 1. With adaptor functions, we want to wrap inside the argument not around the function.

After a fix

#	Source	Compiles to	status
1	each($.users, post($.name))	each(state => state.users, state => post(state.name))	Problematic ⛔️
2	fnIf(Object.values($.results))	fnIf(state => Object.values(state.results))	Works ✅

2 get's fixed but then 1 get's broken.

Problem.

When a function call is an argument to a top-level function call. We don't know whether it's an adaptor function or not.

1. Adaptor Function - we mostly don't want to unwrap state at the top level. (look at 1)
2. Not an Adaptor Function - we want to unwrap state at the top level. (look at 2)

[josephjclark]

Great catch @doc-han. Damn and blast.

I didn't expect this to be such a difficult issue!

So we're sort of back to: walk up to the owning operation function and wrap that. But of course we don't have a good way of knowing what an Operation is 🤦

[josephjclark]

So to make this consistent we're just going to have to have a clear rule.

The rule could be: lazy state resolves to the nearest function, and you the human need to ensure that the nearest function is an operation.

Which is how it works now, right? And I think that covers all the other expression cases, which all resolve to the top operation

// These all resolve expressions to the top level function call
get($.data.url)
get({ url: $.data.url })
get(`${$.data.url`})

// These all resolve expressions to a nested operation (which is totally fine)
each($.data, get($.data.url))
each($.data, get({ url: $.data.url }))
each($.data, get(`${$.data.url`}))

// This doesn't work because the nearest function is not an operation - user error!
fnIf(Object.values($.results))	

I think this works, it's just a question of getting the language right/ Which sounds like a problem for me 😢