plop icon indicating copy to clipboard operation
plop copied to clipboard

[enhancement] Allow action function to return a Promise

Open RobinKnipe opened this issue 4 years ago • 7 comments

Similar to #103, but an extension.

It would be great if a custom action could return a Promise that resolves to an array, rather than just an array.

I've been working on a moderately complicated workflow that is ~50 questions (but could be many more if I ever get a recursive prompt to work properly). With such complexity prompt bypassing is obviously MASSIVELY advantageous, and to facilitate that, one of the first actions to run captures the answers in a JSON file so they can be reused later. I have therefore found my application evolving into 2 distinct phases; part 1 to offer to load an old session file, part 2 to perform the meat of the plop. Having tried various approaches using a single plopfile, it seems I must double-plop. I can get the answers to the initial plop and then trigger the second plop using node-plop to load another file from a custom action, but there are drawbacks to this approach. I am currently having to return an empty action array from the custom action, because asynchronous work needs to be done to determine what will happen next. Instead if I could bundle that work into a promise that later resolves to an action array, that would be much neater!

There is also the potential question: would it be worth extending the plop API to add a concept of "phases", so that multiple plops could be handled by the same config-style plopfile? That way one plop phase could feed its answers and action results into the next plop phase.

RobinKnipe avatar Sep 25 '20 22:09 RobinKnipe

OK so first off - I jokingly love the idea of double-plop in a deep echoy voice of doooooom.

But also, ignoring that - I think we actually have a system in place now that might be able to handle this okay. Can you do me a favor and:

  • Clone the node-plop repo
  • Clone plop repo
  • Change this single LOC:

https://github.com/plopjs/node-plop/blob/master/src/generator-runner.js#L136

To:

return await action(data, cfg, plopfileApi)
  • npm link in the node-plop folder
  • npm link node-plop in the plop folder
  • npm link the plop folder

Then you can run plop locally with your LOC change.

Let me know if updating the action to an async function actually works! If it does, we can add tests, do a bit of documentation, and publish that change

crutchcorn avatar Nov 27 '21 20:11 crutchcorn

Link to related PR: https://github.com/plopjs/node-plop/pull/213

crutchcorn avatar Dec 03 '21 16:12 crutchcorn

I also have this issue (in my case, using fs.readdir inside an action function). I made the change recommended above, as well as changed index.d.ts line 137 to include the promise:

export type DynamicActionsFunction = (data?: inquirer.Answers) => Promise<ActionType[]>;

This fails with:

[ERROR] Package has invalid actions

It appears line 48 of generator-runner.js needs an await:

if (typeof actions === 'function') { actions = await actions(data); }

This allows it to work, though I don't have much runtime on this yet so there could be other issues lurking...

One thought - this is a breaking change. You might want to check if the function is a Promise and add conditional logic accordingly to avoid breaking. Basically a MaybePromise.

cmidgley avatar Dec 20 '21 23:12 cmidgley

@cmidgley apologies for late reply - I've been working on Plop 3 and monorepo conversation - would you potentially be willing to open a PR with those changes (ideally with tests in place so I can see example usage) so that we can discuss them further?

crutchcorn avatar Apr 26 '22 00:04 crutchcorn

Great to hear you are working on plop 3. I ended up forking Plop and making a number of changes to meet my needs, and it's been working great for me. Seeing as I'm 580 commits behind (!), I'm going to leave things as there are until there is some reason why I need to change things.

The change for async is just as I suggest above - literally just the single word async on top of the other suggested changes. Simple if you are interested, but sorry - I'm not going to put together a fork / tests / pull at this time (need to focus on my project). Thanks for your work on this - it's worked out to be a great framework for my scaffolding!

cmidgley avatar Apr 26 '22 01:04 cmidgley

No worries @cmidgley! I looked through your forks and it seems like those indeed are the only two changes made.

If this actually solves problems for you actively, I'll throw together something tonight and publish as a patch version so you can move back upstream :)

Outside of that, was there anything else you'd changed that I might've missed that we can get merged upstream for ya?

crutchcorn avatar Apr 26 '22 01:04 crutchcorn

Thanks for checking my fork! I'd forgotten how much work was in Plop, vs. creating a bunch of custom generators. It has been working flawlessly (async). I use it nearly daily, across around 20 different projects.

Were there any breaking changes to the API? If not (or not significant), then perhaps I can join the club again!

cmidgley avatar Apr 26 '22 10:04 cmidgley