Fix programmatic method invocation.

[Pavel910]

@eahefnawy this PR contains a small fix that was breaking the programmatic invocation of custom methods on the @serverless/template component. CLI usage however was working normally.

I also added a README file with some examples as I see a lot of people struggling with programmatic usage of this component.

This has been tested manually for both CLI and programmatic usage, all the cases I could think of.

[barrysteyn]

Why is this happening? I would suspect that properties from a promise are bleeding through here. However guarding against then does not guarantee that other things may also cause trouble. I suggest that createCustomMethodHandler should be renamed tryCreateCustomMethodHandler and if a component cannot be found, instead of creating an error, rather just return obj[prop]

[Pavel910]

Unfortunately that is not possible, since you're passing template path at method invocation and not Template component construction.

[barrysteyn]

Why is it important to make sure the template is an object? Can this be explained in this comment?

[barrysteyn]

On line 349 above, is it prudent to throw an Error? Perhaps if there was a proper testing framework that would guarantee the behavior of this, however since there is no framework and this is a core part of serverless components, I suggest not throwing an error.

[Pavel910]

template input of Template component can accept both string (path to file) and an object, which you can prepare in advance and simply pass for execution.

[barrysteyn]

@eahefnawy This is fixing a breaking change, so it is super important to be addressed. @Pavel910
Can I make the following comments:

1. Throwing an error in createCustomMethod may be appropriate, but without any testing, changes like this can (and in fact, this change does) cause unintended consequences.
2. Perhaps unit/integration tests on the core libraries. Not saying that things need to be tested vigorously but it may be a good idea (in fact, I will open an issue for this).
3. Thanks very much for the README. It is super helpful.

EDIT: Here is issue for testing https://github.com/serverless/components/issues/511

[Pavel910]

@barrysteyn throwing there is ok because there is nothing else that can be executed if there is no custom method (read my comment in the review section).

To make this component better overall, the template path should be passed to constructor, and not to methods themselves (then your proposed solution with trying would work nicely), but then it is a major change that also involves updates to CLI. That's something @eahefnawy must allow us to do :)

[barrysteyn]

To make this component better overall, the template path should be passed to constructor, and not to methods themselves (then your proposed solution with trying would work nicely), but then it is a major change that also involves updates to CLI. That's something @eahefnawy must allow us to do :)

It seems like this may be the way to go. However such a big change may need to wait for testing (sorry I am going on about testing, but it seems tough to make any change to a core library without testing).

[Pavel910]

@barrysteyn Agreed, it would be nice to have at least basic tests in place.

[eahefnawy]

Thanks folks. Testing is absolutely planned. We just didn't 100% settle on the API yet and currently taking feedback. PR like these are exactly the feedback that we need.

LG2M 😊