Queue and batch dependency actions

[superhawk610]

Related Issue: #33
Related PR: #103

Summary:
Currently, installing/updating/deleting a dependency locks that project and blocks further actions until it has completed. This is a huge hindrance to common tasks like installing multiple dependencies, but due to the way package managers work it's generally a bad idea (read: impossible) to perform multiple yarn adds or the like in parallel.

This PR changes the way dependencies are installed/uninstalled - projects are never locked within the UI, instead creating queued actions when the user attempts to change a dependency while another dependency action is aleady in progress. Since multiple dependencies can be installed/uninstalled with a single command (yarn add foo bar baz), we can batch together queued actions while we wait for the active commands to complete.

The major breaking changes in this PR include:

• queue.reducer.js: This reducer takes the place of package-json-locked.reducer.js. When an action would change a dependency while another is already in progress, it is instead pushed into that project's queue. (NOTE: if you still need to check if that project's package.json is locked, this reducer exports a selector getPackageJsonLockedForProjectId that functions exactly as it did previously.)
• Dependency Action Restructure: All dependency-related actions have been restructured (and a few new have been added) as follows:

// indicate that the user would like to change a dependency
export const ADD_DEPENDENCY = 'ADD_DEPENDENCY';
export const UPDATE_DEPENDENCY = 'UPDATE_DEPENDENCY';
export const DELETE_DEPENDENCY = 'DELETE_DEPENDENCY';

// spawn a child process to actually change the dependencies
export const INSTALL_DEPENDENCIES_START = 'INSTALL_DEPENDENCIES_START';
export const INSTALL_DEPENDENCIES_ERROR = 'INSTALL_DEPENDENCIES_ERROR';
export const INSTALL_DEPENDENCIES_FINISH = 'INSTALL_DEPENDENCIES_FINISH';
export const UNINSTALL_DEPENDENCIES_START = 'UNINSTALL_DEPENDENCIES_START';
export const UNINSTALL_DEPENDENCIES_ERROR = 'UNINSTALL_DEPENDENCIES_ERROR';
export const UNINSTALL_DEPENDENCIES_FINISH = 'UNINSTALL_DEPENDENCIES_FINISH';

// add a dependency to a project's queue
export const QUEUE_DEPENDENCY_INSTALL = 'QUEUE_DEPENDENCY_INSTALL';
export const QUEUE_DEPENDENCY_UNINSTALL = 'QUEUE_DEPENDENCY_UNINSTALL';

// start working on the next action in the project's queue
export const START_NEXT_ACTION_IN_QUEUE = 'START_NEXT_ACTION_IN_QUEUE';

• Non-blocking UI: Buttons that trigger dependency changes are now always enabled, and don't need to check whether the current project's package.json is locked since that logic is handled by dependency.saga.js.
• Testing: All relevant components with testing have been updated to pass with these new changes.

[superhawk610]

While all tests are passing, I am not actually able to test functionality on Windows until a few breaking bugs are fixed (#176).

Flow typing is also broken - I probably need to add a separate Dependency type used specifically during installs/uninstalls with all optional properties, as most of the errors stem from dependencies missing required properties.

[superhawk610]

While I'm thinking about it, I need to update the dependency pane UI to reflect the new queued- statuses. Probably also worth changing button next from Add to Project to Queue ?

[joshwcomeau]

@superhawk610 The breaking bugs should be fixed, hope it's testable now!

While I'm thinking about it, I need to update the dependency pane UI to reflect the new queued- statuses. Probably also worth changing button next from Add to Project to Queue ?

Hm, I think "queue" would be confusing for users, since they probably aren't thinking about it in terms of queueing an install. I think it can stay as "Add to Project", or maybe "Install", since install implies it'll take some time?

[melanieseltzer]

Totally ignore this if you're already aware (cause not sure what you're still working on since it's WIP), but I'm getting some errors when testing the functionality when clicking 'Add to Project' and also 'Delete'. The project seems to instantly get added to the dependency list, that seems... not right. And Delete fails entirely with the same sort of error message.

uncaught at addDependency TypeError: Cannot read property '#<Object>' of undefined

uncaught at deleteDependency TypeError: Cannot read property '#<Object>' of undefined

[superhawk610]

@melanieseltzer yeah, UI integration was the WIP part 😁

If it makes you feel any better, I just flat-out don't have a dependencies pane in my current revision, so it's safe to say there's a bit left to do there. Thanks for the gif, that gives me a good place to start.

[superhawk610]

Alright, the UI is up to date with the rest of the PR and it should be entirely functional.

The only visual bug, the Delete button in the dependency management pane, is detailed in #187.

[melanieseltzer]

@superhawk610 Sick 👌 I haven't had a chance to look at the code, but one thing I noticed with the UI is that if I queue up three or more installs, once the first finishes installing and the spinner stops, the remaining will immediately all at once switch to 'Installing' instead of maintaining the queue.

[melanieseltzer]

And some stuff related to #187 I think...

[superhawk610]

once the first finishes installing and the spinner stops, the remaining will immediately all at once switch to 'Installing' instead of maintaining the queue.

That's intended 😊 Instead of installing all dependencies serially (yarn add foo, yarn add bar), they are batched and installed in parallel (yarn add foo bar).

[joshwcomeau]

Omg this is great 🎉🎉🎉🎉🎉🎉🎉Haven't looked at the code yet, just tried it locally, and it's such a better experience omg.

One UX thought: I wonder if we need to expose the idea of a queue at all to the user... Right now there's a clear difference between a dependency queued to install, and one installing. As you're all too familiar, there's a good reason for this difference, but I'm not convinced the user has to know. Maybe we should treat all queued and installing dependencies the same, saying that they're all "installing"?

The one weird thing about that is that they'll finish at different times (the very first dependency will finish, and then all the rest in a batch), but I think that's ok!

But yeah, no change necessary. I'm not convinced my suggestion is good (I'd love to hear from @melanieseltzer and others!), and even if it is, it's easily something that can be addressed later in another PR.

I also noticed the issues that @melanieseltzer brought up; there's some weird double-button thing happening when it's queued, and a similar issue when it's deleting.

[joshwcomeau]

Phew! Made it through.

I didn't review the tests very thoroughly, but otherwise I gave it a good pass, ran everything locally. Overall it looks great!

I did have some questions and some suggested changes, though. Let me know if any of my suggestions aren't practical.

The only bugs I found were the ones Melanie caught. I think they can be dealt with in a subsequent PR though, shouldn't be blocking.

Excited to see this land, it's such a better experience!

[joshwcomeau]

Yayyy this looks great! Thanks so much for making all the requested changes :)

[joshwcomeau]

Yay, you caught that my suggestion was missing the Array<> haha