feat: Generator rollbacks

[Josh-Walker-GM]

Would close #82 (the oldest open issue!)

Problem
If any error occurs during the generators then the actions already performed are not undone. This can leave a users project in a broken state. It is also more obvious to a user that the command failed if it rolls back its actions.

Solution
Add a rollback feature to the generator commands. This automatically reverts all actions of the generator when an error occurs.

Changes

1. Adds a lib/rollback.js which holds an object stack, these objects are used to either undo a file change or invoke a function during a rollback.
2. Minor changes within lib/index.js file to ensure that when files are written to then a corresponding rollback entry is added.
3. Minor changes to the generator files and helper file. Before a generator is run the rollback stack is reset. The listr task is modified to include rollback functions which will execute the rollback stack when an error is throw from within any task. Any task which contains a non-file change which needs to be undone via a rollback is supported by adding a function call to the rollback stack - there are only a few of these in the generators.

Outstanding

1. Testing to ensure the rollback's undo each generator
   I have tested all of them manually. I created a new test project, init'ed git and ran each command with a deliberate error added as a final step. All rollbacks returned the repo to a state where git showed no differences in git status. We do still need proper tests within each command to make sure the actions are undone by the rollback.
2. Currently leaves empty directories ✅
   Empty directories are now deleted. This works for 2 levels so rollback can delete the parent and grandparent directory of a file which is removed. Important to note that if a user already has an empty directory it will be removed if the generator creates files within it and rollback removes them all. Using .keep files will still prevent this as it means the directory won't be empty.
3. Tests for rollback.js
   One additional unit test needs implemented. This involves mock fs actions which aren't currently supported. I think they are sitting in a different PR which is why I don't add them here.
4. Add a --rollback option so rollbacks can be disabled by --no-rollback ✅
   Added the rollback option to all generators, default to true. It may be excessive for some generators which either create one file or fail so a rollback isn't really necessary but it is only a couple lines to add it in anyway. Keeps the generator options/behaviour consistent.
5. A little refactor to rename and tidy stuff ✅
6. Update the docs to include references to the new --rollback ✅

Questions

1. General discussions around approach
2. Perhaps only adding to the top or bottom of the stack is too restrictive. Perhaps it would be best to be able to add a rollback function call at a certain depth?
3. Should we include commands beyond generate, eg setup?
4. What now is the fate of the destroy commands? Should a rollback be added to them?
5. I have only modified the lib/index.js writeFile function to add a rollback entry. Other functions like deleteFile don't add to the rollback stack simply because no generator uses it. Would it be best to add in rollback entries for these other functions which may be needed in the future or leave it as is for now?
6. I just thought "what if a file is changed during the generator command but not by the generator eg a user changing files mid generator" this would probably mean those changes are undone if the rollback were executed. I doubt we care to support this edge case anyway?
7. How best do we communicate this change in behaviour to existing and new users?
8. Currently this does not support rolling back when running yarn rw g types because it is slightly different from the rest. Should we aim to add rollback support to this too? I don't think it is totally necessary because if I understand correctly this works based on current project files and so would not need a rollback ability.

Ping
@cannikin - Since you are the original author of #82

[Josh-Walker-GM]

The things holding me back from writing some tests for rolling back the actions of the actual command handlers are:

1. I know there are some in the works optimisations to the cli tests so maybe I should wait for those
2. None of the generator tests currently use a mocked fs, they use a real one because they need to load the templates and can get away with just testing the object output by the files functions. This means I'd basically be rewriting all the generator tests to use a mocked fs. Fun exercise to practice jest, mocking etc. but I'd probably want confirmation that this might make it in before I started that work.

Just out of interest, was the format of the object returned by files deliberately designed to look like the object taken by fs.__setMockFiles?

[Josh-Walker-GM]

Okay so I guess this is ready for review in the sense that I can't think of anything else which needs to be added or improved (other than the tests which I have mentioned above).

[cannikin]

Hello! Sorry for the delay, Thanksgiving and everything! I'm gonna look at this today/tomorrow.

What direction did you end up going with the tests, did you do a mocked file system? Did all the generate commands have a requisite destroy command, and did they all still work?

[Josh-Walker-GM]

Hey Rob, no worries at all!

I decided not to push on with the testing before I got some feedback on the approach being acceptable since it's going to be a little bit of work to change all the tests I think.

As for the approach, I noticed all the file io work was ultimately funnelled through one single writeFile function. By adding a line in that function to store the state of the destination before it's written to it's possible to revert all those changes. When it comes to non file based changes, it's possible to add the corresponding undo action to the stack of actions which the rollback executes. Hopefully that's an accurate high level description of what I've attempted here.

I haven't tested the destroy commands and they aren't used for the rollback actions.

[Josh-Walker-GM]

Hey @cannikin any thoughts on the approach used here? No problem if you've not had a change to review. If there's positive reaction then I can go ahead and see about updating the testing setup.

[cannikin]

Haven't had a chance to review, will take a look next week!

[cannikin]

That first failure in the actions checks, can you run this command and see if anything changes in the text fixture, other than the migrations, and a couple scenario files?

yarn build:test-project --rebuild-fixture

The migrations always change and the scenarios get new string values, so you can ignore changes to those files. But if anything else changes it should get committed into the repo!

[Josh-Walker-GM]

No significant changes when rebuilding the test-project fixture. The usual migrations and scenarios (I've got another PR sitting which prevents that 😉). There's a minor bump to prettier-plugin-tailwindcss but I don't think that's important here.

[cannikin]

Looks great, really elegant solution! Thanks again!

[Josh-Walker-GM]

@cannikin should we make a note to try and add some tests for this?

[cannikin]

The suite on the rollback code itself is great...you mean on the actual generators? Yeah I mean I dunno if we need a full test suite for every one, maybe just as simple one like the scripts generator, or one that has at least one custom function? Doesn't seem like it'll be very fun though, mocking out the file system. :(

[Josh-Walker-GM]

Okay that's fine then, thanks for confirming. I did recently think it might be possible to do testing like this a little easier in the e2e tests instead of the unit tests but we'll see what the future brings.