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

Throw if name is empty #25

Open
volkanunsal opened this issue Feb 24, 2021 · 7 comments
Open

Throw if name is empty #25

volkanunsal opened this issue Feb 24, 2021 · 7 comments
Labels

Comments

@volkanunsal
Copy link

volkanunsal commented Feb 24, 2021

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
Copy link
Owner

fogfish commented Feb 24, 2021

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
Copy link
Owner

fogfish commented Feb 24, 2021

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
Copy link
Author

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
Copy link
Owner

fogfish commented Feb 24, 2021

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
Copy link
Author

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

Yes, exactly.

@fogfish
Copy link
Owner

fogfish commented Feb 25, 2021

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
Copy link
Author

volkanunsal commented Feb 25, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants