Hmmm. Forgetting this promise like this doesn't seem great to me. Wouldn't this trigger an "uncaught promise warning" if the initial deployment fails?

Can you try returning this promise from function, and see if that helps?

I changed it to what I think you're asking for. My thoughts here are that it feels wrong to make the 'ready' callback async because during that time we would lose track of file changes -- is my instinct there correct?

I also tested my own CDK stack (that fails deployment) with void handleFileChangeEvent and I received the same "failed to deploy" message that I was expecting:

My thoughts here are that it feels wrong to make the 'ready' callback async because during that time we would lose track of file changes -- is my instinct there correct?

Can you check what the behavior is manually, and report back here? 🙂

So here's what I found:

You can specify async callbacks, and typescript will not yell at you, but they will be ignored. see this issue. For example, even though the handleFileChangeEvent function is async, no one is actually waiting for the event to finish. Same goes for the callback at the 'ready' event.

As to any unhandled promise warning, I think we are in the clear. It seems related to how we are catching errors in invokeDeployFromWatch anyway -- we ignore them, and deploy will show them (as I have confirmed locally when using void handleFileChangeEvent()).

All that to say, I think we are fine with void handleFileChangeEvent() we are also fine with return handleFileChangeEvent(). In both cases, a failed deployment will show on screen as expected, with no unhandled promises.

We could even do await handleFileChangeEvent(). That would result in the same behavior.

With that in mind I've changed the function to be async and we await handleFileChangeEvent(). But it is an inconsequential change. Can confirm that the behavior for this is as expected on my local computer -- I get the deployment failure or success message printed out. And watch is immediately listening for file changes (not waiting for first deployment to finish).

One thing I think we can improve here is that the extracted function is too big. We need only the deployment part (when latch is 'open'), but we extracted all of it, even the parts that we don't need. That's why we have the awkwardness with the filePath having to be undefined.

So, I think the way to go is something like this:

    chokidar.watch(watchIncludes, {
      ignored: watchExcludes,
      cwd: rootDir,
      // ignoreInitial: true,
    }).on('ready', () => {
      latch = 'open';
      debug("'watch' received the 'ready' event. From now on, all file changes will trigger a deployment");
      print("Triggering initial 'cdk deploy'");
      await deployWhileObservingForFileChanges();
    }).on('all', async (event, filePath) => {
      if (latch === 'pre-ready') {
        print(`'watch' is observing ${event === 'addDir' ? 'directory' : 'the file'} '%s' for changes`, filePath);
      } else if (latch === 'open') {
        print("Detected change to '%s' (type: %s). Triggering 'cdk deploy'", filePath, event);
        await deployWhileObservingForFileChanges(); // this name TBD of course
      } else { // this means latch is either 'deploying' or 'queued'
        latch = 'queued';
        print("Detected change to '%s' (type: %s) while 'cdk deploy' is still running. " +
            'Will queue for another deployment after this one finishes', filePath, event);
      }
    });

And deployWhileObservingForFileChanges is:

      async function deployWhileObservingForFileChanges() {
        latch = 'deploying';

        await this.invokeDeployFromWatch(options);

        // If latch is still 'deploying' after the 'await', that's fine,
        // but if it's 'queued', that means we need to deploy again
        while ((latch as 'deploying' | 'queued') === 'queued') {
          // TypeScript doesn't realize latch can change between 'awaits',
          // and thinks the above 'while' condition is always 'false' without the cast
          latch = 'deploying';
          print("Detected file changes during deployment. Invoking 'cdk deploy' again");
          await this.invokeDeployFromWatch(options);
        }
        latch = 'open';
      }

currently named depoyAndWatch, hopefully that is reasonable.