plopjs / plop

Consistency Made Simple
http://plopjs.com
MIT License
7.07k stars 275 forks source link

[enhancement] Allow action function to return a Promise #242

Open RobinKnipe opened 3 years ago

RobinKnipe commented 3 years ago

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.

crutchcorn commented 2 years ago

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:

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

To:

return await action(data, cfg, plopfileApi)

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 commented 2 years ago

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

cmidgley commented 2 years ago

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.

crutchcorn commented 2 years ago

@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?

cmidgley commented 2 years ago

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!

crutchcorn commented 2 years ago

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?

cmidgley commented 2 years ago

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!