fogfish/aws-cdk-pure

Throw if name is empty

Opened this issue · 7 comments

Leaving the name empty is a frequent source of bugs and errors, since I use lambda functions, and forget to add the name field. e.g.

pure.join(scope, cloud.stack(() => ({}))) 

Would it be possible to create an invariant to check the name field? And even better would be if I can skip the empty lambda altogether:

pure.join(scope, cloud.stack('MyStack')) 

It would remove a lot of boilerplate.

Thank you for raising this!

Unfortunately, this is a limitation of JavaScript, anonymous/lambda function do not have name attribute
https://github.com/fogfish/aws-cdk-pure/blob/master/pure/src/index.ts#L60

const MyFun = (): ddb.DynamoProps => ...
pure.iaac(ddb.Dynamo)(MyFun) // the name attribute is define to `MyFun`

pure.iaac(ddb.Dynamo)((): ddb.DynamoProps => ...) // the name attribute is not defined

This is not nice, I'd like to use anonymous (lambda) functions too. It simplify the design.
The function iaac allows you to define a name but I would not recommend it (const MyFun + pure.iaac is really the same)

pure.iaac(ddb.Dynamo)((): ddb.DynamoProps => ..., 'MyFun') 

Unfortunately, I do not have a perfect solution to use pure anonymous (lambda) functions. I can introduce any number of functions to lift lambda to IPure type but as I said earlier syntactically it would be same as const MyFun, e.g.

const MyFun = (): ddb.DynamoProps => ...
pure.iaac(ddb.Dynamo)(MyFun)

// vs

pure.iaac(ddb.Dynamo)(pure.lift('MyFun', (): ddb.DynamoProps => ...))

I hope I've understood your issue properly. If not could you please share code snippet with a problem.

Speaking only about this code:

export function iaac<Prop, Type>(f: Node<Prop, Type>): (pure: IaaC<Prop>, name?: string) => IPure<Type> {
  return (pure, name) => unit(
    (scope) => new f(scope, name || pure.name, pure(scope))
  )
}

Indeed, it make sense either to disable a type of anonymous function or raise exception if name is not defined to prevent bugs. I'll definably improve this block with better error handling.

I've tried to make the function input polymorphic in my code. This is my solution (WIP)

export declare function iaac<Prop, Type>(
  f: Node<Prop, Type>
): (pure: IaaC<Prop>, name?: string) => IPure<Type>;
export declare function iaac<Prop, Type>(
  f: Node<Prop, Type>
): (name: string) => IPure<Type>;
export function iaac<Prop, Type>(
  f: Node<Prop, Type>
): (...args: unknown[]) => IPure<Type> {
  return (...args) => {
    let id!: string;
    let pure: IaaC<Prop> = () => ({} as Prop);

    if (typeof args[0] === 'string') {
      if (!args[0]) {
        id = args[0];
      }
    } else if (typeof args[0] === 'function') {
      pure = args[0] as IaaC<Prop>;
    }

    if (typeof args[1] === 'string' && !args[1]) {
      id = args[1];
    }

    if (pure.name) {
      id = pure.name;
    }

    if (!id) throw new Error('Id is required.');

    return unit((scope) => new f(scope, id, pure(scope)));
  };
}

The problem I'm running into is this error:

Overload signatures must all be ambient or non-ambient.ts(2384)

This looks a bit ugly, but it would allow for a very simple end user API:

pure.join(scope, cloud.iaac(Stack)('MyStack'))

Are you trying to make a construct that takes empty (aka default properties)?

One idea, just make type (pure: IaaC<Prop>) => IPure<Type> composable.

const MyStack = cloud.iaac(Stack)
pure.join(scope, MyStack)

Are you trying to make a construct that takes empty (aka default properties)?

Yes, exactly.

Could you please clarify your usage of "empty properties"? In my life with AWS CDK, I've never used 'em? I've always needs to defined for props to resources.

The most common usecase for me is the Topic class. All the properties are optional. If you don't provide a displayName, one is generated for you.

These are the constructs where I used empty properties:

topic
que
logGroup
fargateTaskDef
logGroup
stack