Flags should have consitent api

[theoboldalex]

PR closes #1628 and is a follow on from the initial implementation of the --path flag merged in #1627

Provides a suite of CLI flags for local invocation of Bref event driven functions when not utilising Serverless framework. This makes the API for invoking event driven Bref functions consistent with running local functions using serverless bref:local

Available flags

Flag	Description
-f	The handler file
--path	A path to a valid JSON file containing event data
--data	A JSON string containing event data

Backward compatability

Backward compatibility with the current way to invoke bref-local has been maintained so calling via vendor/bin/bref-local <handler> <event-data> will continue to work. Essentially, flags are 100% opt in.

Ordering of flags

The order of flags is now irrelevant and any combination of either -f && --path or -f && --data will be accepted as valid invocations.

Tested commands

I have tested invoking functions using bref-local in the following combinations.

Backward compatible commands

✅ $ vendor/bin/bref-local index.php -> Hello World

✅ $ vendor/bin/bref-local index.php '{"name": "Fred"}' -> Hello Fred

Flag based commands

✅ $ vendor/bin/bref-local -f index.php -> Hello World

✅ $ vendor/bin/bref-local -f index.php --path event.json -> Hello Fred

✅ $ vendor/bin/bref-local -f index.php --data '{"name": "Alex"}' -> Hello Alex

✅ $ vendor/bin/bref-local --path event.json -f index.php -> Hello Fred

✅ $ vendor/bin/bref-local --data '{"name": "Alex"}' -f index.php -> Hello Alex

[theoboldalex]

Thanks for the review @shadowhand

I have cleaned this up based on your suggestions.

[theoboldalex]

Any thoughts on this one @mnapoli ?

[mnapoli]

Hey, sorry for the delay. Big PRs are always harder to merge than simple ones 😬

There was a misunderstanding on my side, that's my fault. In the initial conversation, you mentioned 2 changes:

In the current implementation, when we pass the --path flag, it must be passed before the handler file.

When running Bref with Serverless, the handler file has the -f flag. I could add this to the non-Serverless bref-local script to ensure consistency in how functions are invoked locally regardless of whether we are using Serverless, CDK or Terraform.

I only considered the first suggestion, and I think solving this "bug" is great.

Regarding the second proposition, I'm not convinced. Yes, having consistency in flags could be good in theory, but:

• those that use bref-local are usually not using Serverless Framework, so consistency with SF is not important
• the SF API sucks IMO. I've been passing those -f function-name arguments since forever and I hate it

The Node world has a poor ecosystem for dealing with CLI flags and arguments. That's why they almost always use options (bref-local --option=x) instead of arguments (bref-local x). In the PHP world, the Symfony Console has set an invaluable precedent with very strong commands/sub-commands/args/options concepts, which always work well. I think we should take advantage of this for a better UX.

So here's why I'm really hesitant to merge this:

• This PR changes the public API (even with backward compatibility), which is something we don't want to do without good reason
• The SF API is worse, and aligning on this for consistency is not important and brings a worse UX for Bref users
• This PR changes logic that's not heavily covered by tests, so I'm always even more hesitant to merge

All this is adding up. I'm sorry because of the time you invested in the PR, but I don't think it's worth merging in its current state 😬 Hope that makes sense.

I think the best option would be to only solve the 1st problem (--path should work if passed before or after arguments), if that's possible?

[theoboldalex]

Thanks for the detailed feedback. @mnapoli

I'm not sure if it is possible to solve just the first issue using getopt but I can certainly take a look to see how feasible it is without any dependencies. I'll do some investigating and hopefully get some changes up for your consideration. Thanks 🙏

[mnapoli]

Closing inactive PRs, feel free to reopen.