Add Decorator Support

[alexlafroscia]

trafficstars

This adds two decorators, memberAction and collectionAction, that are exported from ember-api-actions/decorators. They are built to be compatible with the V1 decorator spec.

The exported functions are called with the same options as the existing helpers, and internally call the exact same logic.

Open Questions:

• [x] Is it worthwhile to modify the existing functions instead to detect usage as a decorator, as to avoid exporting a second set of helper methods?
• [x] Should the documentation be updated to default to the decorator-based usage?

Left To-Do, Pending Question Resolution:

• [x] Add documentation about decorator usage

Closes #413

[alexlafroscia]

@dbollinger I'd love to get your thoughts on the "open questions" above -- especially whether we should detect usage inside one version of memberAction and collectionAction or keep them as two separate imports that are designed for specific purposes.

[dbollinger]

This is great!

For question 1 -- I generally favor the advice to "prefer duplication over the wrong abstraction" and in this case a separate export seems fine. Alpha/beta releases can help make that a smoother process as well.

For question 2 -- I think some additive documentation would be a good first step, but I'm not sure about the best way to manage that alongside the release process.

[alexlafroscia]

prefer duplication over the wrong abstraction

Good words to live by! I’m cool with making it a separate import right now and maybe combining them sometimes in the future. This package is technically still pre-1.0.0, so the API can break before an official 1.0.0 release if it needs to!

Given the fact that it won’t be the exports from the “main” module, I’ll add some README documentation to for the decorator usage rather than updating the existing documentation to use the new flavor.

[codecov[bot]]

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@ff24733). Click here to learn what that means. The diff coverage is n/a.

[alexlafroscia]

Rebased to use setupPretender again

[alexlafroscia]

Hmm... So it seems like the decorator tests don't working for Ember... 2.16 and 2.18.

Some thoughts:

• Should we bother with those Ember versions at all? We could raise the minimum version run against to the most recent LTS going forward
• We could just skip the decorator tests for these old Ember versions

[emilkarl]

Hmm... So it seems like the decorator tests don't working for Ember... 2.16 and 2.18.

Some thoughts:

• Should we bother with those Ember versions at all? We could raise the minimum version run against to the most recent LTS going forward
• We could just skip the decorator tests for these old Ember versions

Will decorators work in those versions anyways? 🤷‍♂️

[alexlafroscia]

Will decorators work in those versions anyways? 🤷‍♂️

They could -- the original implementation of the decorator I had did -- but the way it is not implemented, where TypeScript's compiler is satisfied, does not.

I would be in favor of just dropping support for these two very old Ember versions and increase the minimum version to 3.12. I will address that in a separate PR and we can discuss the idea more there.

[esbanarango]

@mike-north @alexlafroscia do we know what's the status here 🙏

[alexlafroscia]

No updates from me 🤷‍♂️ I honestly forgot I made these PRs, but with the update-to-Ember-3.12-at-a-minimum PR still standing as it is, I don't think we're in a place to move forward really.

[ducharmemp]

Looks like that Pr landed so this PR can move forward? I think the prototype workaround is alright but decorators would also be nice!

[MarcoKramer]

Is there any news on this? Would indeed be nice to have this in.

[snewcomer]

@alexlafroscia Possible we could rebase with the latest on the main branch and see if @Turbo87 would be open to merging and releasing it?

[esbanarango]

That would be 🔝 @alexlafroscia @snewcomer !! Thank you for putting this together 🙏

[jkeen]

@snewcomer Did the rebase and made a new PR which is here: #609

[Turbo87]

FWIW we (@mainmatter) forked the addon and created a new API that does not involve decorators at all but should also be a bit more TS friendly once we add types: https://github.com/mainmatter/ember-api-actions

[Turbo87]

also, even if we merged this PR we still couldn't release it, because @mike-north still hasn't given anyone npm ownership of the package... :-/

[jkeen]

@Turbo87 Thanks for forking and taking over! I hate it when this happens to great projects (and great PRs) and there's no clear passing of the torch.

Your solution feels great, and I'm gonna migrate over to that and forget about this branch entirely! 🎉

[jkeen]

@mike-north Pssssst think you could pass the keys over to someone else and archive this repo, maybe updating the readme with a link to the maintained project? Seems like you're not into this project anymore and it'd be great to give the community a direction to go towards.

[jkeen]

In trying to use your version @Turbo87, it looks like…

a) it's not actually a fork of this project but a completely new project with the same name? If you're looking to carry this project forward into the future (and I hope you are!), it feels weird to not have the years of git history that came before. If this were my project that I started and then abandoned, I'd feel slighted if my name were erased from the history.

b) Your addon seems to be missing some key features that this one has, like the before and after hooks on the actions. Was that intentional?

[Turbo87]

it doesn't have the history because we've rewritten it from scratch with a completely different API 😅

yes, before and after missing is intentional, because you can just do those before and after the action call within the async function.

also, I apologize for the lack of documentation so far. it's still work in progress, but I thought I'd share it here already anyway :)