fogfish / aws-cdk-pure

Purely Functional and high-order cloud components with AWS CDK
MIT License
94 stars 6 forks source link

Throw if name is empty #25

Open volkanunsal opened 3 years ago

volkanunsal commented 3 years ago

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.

fogfish commented 3 years ago

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.

fogfish commented 3 years ago

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.

volkanunsal commented 3 years ago

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'))
fogfish commented 3 years ago

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)
volkanunsal commented 3 years ago

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

Yes, exactly.

fogfish commented 3 years ago

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.

volkanunsal commented 3 years ago

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