[For Discussion] Add registrations

[ipcrm]

Registrations permit things like dynamic bucket targets, re-using goals for multiple conditions, etc.

[ipcrm]

Can you provide some detail on what you see as the shortcomings to this implementation?

@ddgenome , sure. The challenge with this as written is that you can create a publishToS3 goal that is missing required options and the types don't tell you.

For example, this is valid (meaning the TS will not complain):

const publish = new PublishToS3({
        bucketName: "test",
        uniqueName: "publish-to-s3",
});

But the goal will actually fail because it is missing the required properties.

All other PR comments have been acted on.

[ddgenome]

So I've made a few minor changes to the code but it seems to me there is still something fundamentally wrong with the current model. It seems much of the issue comes from trying to maintain backward compatibility, not having a default implementation, and wanting the types to be as restrictive as possible. I've been thinking about how to satisfy all the constraints and the only way may be to introduce two new types. I'll try to put something together showing that.

[ddgenome]

In thinking about this more, perhaps it is better to defer correctness into the runtime. Perhaps both the constructor and the registration should accept the entire payload, with all properties optional. The execution can then merge the two objects, with the registration's taking precedence, call the callback if provided and then check for validity. You could do some validation in the with method, ensuring all required properties are set if there is no callback provided.

What do you think?

[ipcrm]

@ddgenome I continue to wonder if using registrations is different enough that we shouldn't just move this from 1.x -> 2.x and call it a breaking change? While I agree with you that the validation (should we support both approaches like this branch attempts to) will have to happen at runtime, I'd prefer the types be able to tell when you've not supplied all the required properties during development. If we called it a breaking change, 2.x could simply require registrations and the constructor would only hold goal definition details. If you feel it's not appropriate to move to 2.x, then I agree with your previous comment and can extend the with methods to perform some validation.