OpenFn / kit

The bits & pieces that make OpenFn work. (diagrammer, cli, compiler, runtime, runtime manager, logger, etc.)
10 stars 9 forks source link

Allow state operator to be a simple alias #660

Closed josephjclark closed 5 months ago

josephjclark commented 5 months ago

Right now the state operator only works if it's used inside an argument to an operation.

This works:

post('url', {
  name: $.data.fullname,
});

This does not:

post('url', (state) => {
  const data = {
    name: $.data.fullname, // error!
  }
  return data;
});

This makes sense to me, but I think it might hard for our users to understand.

I wonder if we can use $ as an alias for state in these cases.

If $ is used inside a scope where state is declared, it's probably safe to alias in any expression or on the left of an assignment.

But if there's no state in scope (or you're leaning on the global state) you'll probably get in trouble. I think in those cases we should throw an error.

But with simple $ aliasing, these should all be fine:

fn((state) => {
    $.count = 1;
    const name = $.data.name

    return $;
});
josephjclark commented 5 months ago

When I left my desk yesterday I was sure that we could do this, allowing a simple alias which doesn't really do anything but lets people use $ how they might expect.

When I went to bed I wasn't sure. Aren't we just enabling bad practice here? Diluting the meaning of $? And to what end?

This morning I'm convinced of that.

$ is not an alias for state. It is not a variable that you can assign or assign to. It does not mean you don't have to declare the state parameter anymore.

$ is a lazy state operator. You can only use it where you can use a (state) => { ...} function (a open function or a reference).

By pretending it's a state alias, like it's a variable that you can use like normal, we're just making the picture more confusing. Like it's sort of a variable, sometimes, but not really? I don't know how to explain that, it would just be a magic symbol that sometimes works and sometimes doesn't.

I prefer to explain it as an operator or even placeholder which expands/resolves, on demand/just in time, to read a path from state. That's it.

I will update the docs PR and slightly downgrade the wording in the patch notes here. I don't think this is ready for a big public announcement yet, so I'm just gonna stealth release the improvements.