Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Runtime: allow re-usable functions across a workflow #648

Open
josephjclark opened this issue Apr 3, 2024 · 4 comments
Open

Runtime: allow re-usable functions across a workflow #648

josephjclark opened this issue Apr 3, 2024 · 4 comments
Labels
UX UX and DevX things

Comments

@josephjclark
Copy link
Collaborator

josephjclark commented Apr 3, 2024

The Problem

It's common in workflows to want to re-use the same helper function in multiple steps.

For example, let's say we create our own id-generator or date-formatter, or data converter.

We used to allow functions to be written to state and used in multiple steps. I think that might still work? [citation needed]. It should break in-between steps really. Even if it's works its not a great approach.

Solutions

workflow.functions

Declare a function somewhere on the workflow body. Maybe we can have workflow.fns as a peer of workflow.steps. It probably references a file, rather than defines functions inline.

I'm not keen on this idea because it need s special UI support in Lightning and I just don't think we have the resources.

Make functions global

We could say: if you declare a function in a step, that function will automatically be available to the rest of the workflow.

This would either be through a function declaration function generateId() {} or possibly an exported function declaration (are arrows allowed?). Exported functions would open a door to unit testing too. Of course, we could use compiler magic to auto-export top-level functions.

I have no idea how we'd implement this technically.

Do we have to pre-parse the workflow, find all functions, extract their source, and set them into the runtime context? Sheesh, that sounds like a lot of work.

Alternatively, when a function is declared, we call a special runtime API to register it, and it's then available to all other functions downstream. This would be alright.

Use an API

We could provide a special API like declare() which takes a function declaration and a name as an argument. Then, just like the global functions thing, this function would be made available to all downstream steps.

We'd have to inject the declare function from the runtime, just like we do with the defer function.

This could be compiler magic again - if the declare function is a useful hook for implementation, then maybe the compiler can wrap all top level function declarations into declare cals.

@josephjclark josephjclark added the UX UX and DevX things label Apr 3, 2024
@github-project-automation github-project-automation bot moved this to New Issues in v2 Apr 3, 2024
@josephjclark
Copy link
Collaborator Author

josephjclark commented Apr 3, 2024

Quick brainstorm of options:

  • Allow the runtime to safely serialise functions. OR some functions. You should be able to decompile a function down to its source code, right? How does that work and can we exploit it? Then we can write functions to state and pass them downstream. This is a bit of a fiddly UX though because if you return state downstream, you could lose the functions entirely. Also I think writing logic to a state object feels like an antipattern
  • That means we have to import the custom functions into the job.This might make the compiler's life harder because it has to distinguish adaptor functions from user functions (and what if there's a conflict?)
  • One solution is to use a convention. If there's a file called utils.js - or perhaps a path to a utils file is on the workflow object (yes I like this), we can add import * as util from './utils' into each job. Now the exports from each job are available on a utils object. So you can do utils.convertToLocalCurrency. I quite like this: it's achievable at low cost and adds a lot of utility. We need the right word for utils. Maybe the workflow can map it to whatever variable you like.

@doc-han
Copy link
Collaborator

doc-han commented Dec 5, 2024

Allow re-usable functions across a workflow

I align more with the workflow.functions idea.

Workflow level function

Users have a single place(maybe a file) where they define all their utility functions that can be accessed anywhere in a workflow.
UX - editing/viewing should be accessible on the workflow editor and also on any step editor. since it's global functions. access should be global from the workflow level.

Job level function

When users want to define simple functions to use in a single job.

  • State functions (PR up)
  • Normal function definitions actually work here too. tested!
    You can actually define a function at the top-level of your job code and have it do magic.
    example:
function kebabCase(name) {
    return name.replace(/\s/g, '-')
}

fn(state => {
    state.data.name = "openfn new kit"
    return state;
})

fn(state => {
    state.data.kebabName = kebabCase(state.data.name);
    return state;
})

I think allowing both workflow level and job level functions will be vital.

Other suggestions I don't align with

  1. Make functions global: Hence when a function is defined in a job, we make it global to a workflow.
    • Hoisting functions will be technically complicated
    • Users finding where functions were defined will be like hide and seek.
  1. Using an API (declare): This is to provide a declare api which makes a function declaration available to all downstream steps.
    • I think this still suffers from hoisting and finding where functions were defined.
    • It'll only be cool if it can be visually represented in the workflow editor. Hence users can visually know when a function was defined and the downstream steps that will benefit from it.

@doc-han
Copy link
Collaborator

doc-han commented Dec 5, 2024

Quick brainstorm of options:

  • Allow the runtime to safely serialise functions. OR some functions. You should be able to decompile a function down to its source code, right? How does that work and can we exploit it?

It works by calling toString() on the function. It has to have no external dependencies OR you should know how to figure out what dependencies it depends on.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/toString

@josephjclark
Copy link
Collaborator Author

Good points about hide-and-seek!

I generally agree with you, but I'm really worried about the cost of supporting workflow.functions from a Lightning UI point of view. It really adds a lot of complication there.

But let's dig into it a bit deeper.

Functions object

We could do this:

{
workflow: {
  steps: [],
  functions: {
    kebabCase: () => { /*... */ } ,
    uuid: './js/util/uuid.js',
 }
} 

If we only think about the CLI and workflow.json, this is the most intuitive approach. And to be fair lightning could easily write the functions to the state object nicely.

But it's pretty poor UX. No-one is ever going to write a function directly in json, and having one function per file is also pretty unpleasant. And I guess that file would have to export default or something, which is again a limitation.

But it would at least be easy to inject those functions into the runtime context, so that's a plus.

Functions module

Alternatively I guess we'd do this:

{
  workflow: {
     steps: [],
    functions: './js/util.js'
  }
}

Where functions points to a js module. The module must have named exports. All exports are injected in the runtime global scope.

// utils.js
export const uuid = () => {}
export function kebabCase() {}

Since it's just a js file, the functions are unit testable, so that's nice.

If Lightning were to ever provide a UI for these functions, it could serialize the module into workflow.functions as an inline string. And although we do need UI and database support, really it would just be flat code editor which ultimately holds a single string. So it's not too intricate.

This feels like a better approach?

General assumptions

  • Helper functions do not have the adaptor functions in scope (is this right?)
  • Helper functions cannot import from other libraries
  • Should we inject datefns and lodash into the helper function scope? Probably yes?
  • Helper functions can be operations, if you want, but they don't have to be
  • If there's a name clash between an adaptor function and a util function, what do we do? You'd think we'd just throw an exception,but what if it's useful to monkeypatch and adaptor function? This admittedly sounds like a slippery slope!
  • I suppose I would be a little worried about the creation of workflows which do like runjob() and put ALL the logic, including a whole adaptor implementation, inside the workflow functions. This is obviously very cheeky, but does it actually matter?
  • Helper functions need to run sandboxed by vm, just like job code, and with limited scope. Are there other security functions here? Is generic helper code less safe to run than job code?

Compiler note

If we're going to inject anything in the global runtime context, the compiler needs to know about it. Otherwise when it sees an undeclared global variable, it'll assume it belongs to the adaptor and try and import it, which breaks everything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UX UX and DevX things
Projects
Status: Done
Development

No branches or pull requests

2 participants