Conversation
| } | ||
|
|
||
| export class SayFunction extends Construct { | ||
| fn: Function<InputEvent, void>; |
There was a problem hiding this comment.
I found this particularly painful with all 3 cdk-leaning approaches. Because the function needs to be passed in, instead of being referred to directly, the user has to create appropriate types and cannot benefit from direct type inference.
| super(scope, id); | ||
| this.fn = new Function( | ||
| scope, | ||
| "SayFunction", |
There was a problem hiding this comment.
This is a problem area I've run into in the past. This is going to result in the id path being SayFunction/SayFunction which usually will duplicate the name in the created resource. This is the CDK-recommended way of making a construct, even if it is just one resource, but the double naming leads to a clunky experience.
There was a problem hiding this comment.
This is up to the user though. If the caller determine the meaning, then the caller should call the top level "sayFunction". If the construct determines the meaning, the construct should call it "sayFunction".
In the form case, the construct can just call it "f" or something to leave it up to the caller. Or "Resource" like CDK does.
| import { App, Stack } from "aws-cdk-lib"; | ||
| import { Function, StepFunction } from "functionless"; | ||
|
|
||
| export class MyStack extends Stack { |
There was a problem hiding this comment.
This is a nice hybrid between single file and CDK-leaning but it doesn't actually accomplish the objective of having functions in their own file. Showing it as another option to consider.
| export default Function(async (event: { message: string }) => { | ||
| console.log(event.message); | ||
| return; | ||
| }); |
There was a problem hiding this comment.
This pattern seems promising - almost looks like syntax, e.g. a FunctionDecl
| export const Function = <In, Out>( | ||
| closure: fl.FunctionClosure<In, Out>, | ||
| props?: fl.FunctionProps | ||
| ) => { | ||
| return new fl.Function(stack, __filename, closure); | ||
| }; |
There was a problem hiding this comment.
It's clean, but the singleton stack is a bit of a problem.
There was a problem hiding this comment.
Really shows how easy it is to build a simpler layer on top of our foundation
There was a problem hiding this comment.
Someone didn't read the disclaimer at the top of the file :)
I have ideas on how to make this both flexible and extensible but I scoped them out of the initial sketch.
|
Have we considered the following variation?
export * from "./user-table";
export * from "./handler1";
export * from "./workflow1";
export const app = new App();
export const stack = new Stack(app, "stack");
export interface User {
userId: string;
}
export const userTable = new Table<User, "userId">(stack, "User");
import { stack } from "./stack";
import { userTable } from "./user-table";
export const getUser = new Function(stack, "getUser", async (userId: string) => {
return userTable.get(userId);
});
import { stack } from "./stack";
import { userTable } from "./user-table";
export const deleteUser = new StepFunction(stack, "deleteUser", async (input: { userId: string }) => {
await userTable.delete(userId);
}); |
| export class Workflow extends Construct { | ||
| constructor(scope: Construct, id: string, props: SayFunctionProps) { | ||
| super(scope, id); | ||
| new StepFunction(scope, "Workflow", async (event: { name: string }) => { | ||
| await props.sayFunction({ message: `Hello ${event.name}` }); | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
There is another option here where the step function is sub-classed rather than wrapped. This would get rid of the naming issue.
export class Workflow extends StepFunction {
constructor(...) {
super(scope, id, async () => {
});
}
}There was a problem hiding this comment.
If I was encapsulating logic I would prefer this approach or the functional approach.
In the case of needing multiple interconnected resource, the construct makes the most sense.
There was a problem hiding this comment.
I had a third variation at one point that had this as well. I think the number of way there are to slice this is a part of the reason people complain about CDK being too open.
| const sayFunction = SayFunction(stack); | ||
|
|
||
| Workflow(stack, { | ||
| sayFunction, | ||
| }); |
There was a problem hiding this comment.
nice, I like this, and with an option ID, it can be singleton or duplicated.
|
A subjective observation I found interesting working through this sketch: Adopting a CDK-leaning approach tend to negate the conveniences I like about functionless. It also introduces a bunch of decisions the user must make and form good opinions on. There is a lot of cognitive overhead here, but you get more flexibility. The CDK-less approach is able to benefit from the same conveniences as the single file approach. Additionally, the user doesn't really have to make many decisions. However, it is less composable. As a library writer, I want the CDK-leaning approach. As an application developer, I want the CDK-less approach. I think the ideal would be if we could find a way to make the CDK-less approach extensible in a way that would also facilitate library development. |
I think this technically would work but introduces a circular dependency. This also makes it possible to have more than one resource per file which would breaks some assumptions that would be nice to unblock higher level tooling. Also re-introduces needing to come up with ids again. Edit: We should still include it since it is a viable variation to consider. Will add. |
I agree with this. User land should be as lean as possible with layers for the more advanced people to peel back. Advanced developers are more forgiving of complexity. |
No description provided.