Here's an example of how we could go about making something like createFunction type safe: #62334

However, one question to solve with this is how to deal with the builders if you can do fn.addArg() for each individual argument... how can you enforce that all provided args are present on the function when they aren't added all at once?

how can you enforce that all provided args are present on the function when they aren't added all at once?

I guess I should've rephrased this to "you can't enforce that all provided args are present on the function". So if we let folks update functions with an individual argument instead of passing all of them at once, the tradeoff we are making is that we aren't 100% sure all args have been provided.

The only way around this I can think of would be to force users to provide a full list of the required arguments when instantiating the function builder, and then allow them to manipulate the args one-by-one later. That way you'd know that no required args are missing.

Which brings another question to mind - is there a use case for removing an arg from a function?

the ability to add arguments later and remove them just makes the api way easier to consume (and refacture current string concatenation logic). however i think everything should still be possible with the above mentioned limitations (you need to provide default values for all required arguments and you cannot remove arguments).

what we are discussing here is are we willing to trade typesafety for better usability ? (we could still provide runtime checks if we opt for not being type safe here.

you need to provide default values for all required arguments and you cannot remove arguments

Agreed, IMHO this is an acceptable tradeoff for type safety vs useability. If you are forced to provide defaults for all required args up front, then you have the safety of knowing that you aren't missing anything. And then any subsequent call to update one of those args will enforce the correct type.

We could also try to do runtime checks, the only tricky part there is that it would probably require looking up the referenced functions from the registry to get their metadata, vs the current implementation which is much simpler in that it is static.

I wonder if having separate createX and createXBuilder functions makes this more confusing.

Thinking out loud -- What if we only provided the builders, and at the end when you call ExpressionBuilder.toAst() it goes through all of the functions & calls .toAst() on each of them?

That way you could hang on to references to functions even after they've been added to the expression, and make updates.

For example (I'm using a class-based approach for the builders here, but you get the idea):

import { expressionAstBuilder } from '../expressions/public';
const { Expression, ExpressionFunction } = expressionAstBuilder;

// create instance of expression builder with one function as the "initial state"
const myExpression = new Expression([
  new ExpressionFunction('esaggs', { index: 'abc123' }
]);

// add another function
const fn = new ExpressionFunction('foo', { bar: true });
myExpression.addFunction(fn);

// update the function you've just added
const subExpression = new Expression([...]);
// works cause you saved a reference to it
fn.addArgument('bar', subExpression);

// toAst will go through and call toAst on each of the provided functions
const ast = myExpression.toAst();

// I suppose you could also offer a way to get the string as a convenience.
// It would call toAst() and then convert it to the expression string.
const string = myExpression.toString();