Allow event for manually triggering a reload.

[nicksrandall]

This PR contains:

• [?] bugfix
• [?] feature
• refactor
• tests
• documentation
• metadata

Breaking Changes?

• yes
• no

If yes, please describe the breakage.

Please Describe Your Changes

This change allows the recipe described here to work: https://github.com/shellscape/webpack-plugin-serve/blob/master/recipes/watch-static-content.md

It basically enables the "reload" event to be emitted so that a reload can manually be triggered.

[nicksrandall]

I looked into adding test cases but it looks like there are no tests written for this file yet.

[nicksrandall]

@shellscape do you have any time to review this PR?

[shellscape]

I'm a bit pressed for time this week, and we were out of town for the holiday. It's on my list to take a look at.

[nicksrandall]

PING*

@shellscape, if you are pressed for time, is there another contributor who can look at this tiny pr?

[matheus1lva]

He has been busy @nicksrandall, i have already had a look at this, but only he can publish the library, that is why i prefer him to double check it anyways!

[shellscape]

There's a few unintended consequences to this, and it (and the recipe) raises questions as to why this wasn't working and there was a recipe for it. So there's probably a regression that needs to be tracked down before we merge this. Also need to clone it and verify it works. You've done good work here, and we need to investigate a few things in the project before moving forward.

It's frustrating when a PR takes a while to merge. But not all tiny PRs are truly tiny maintenance issues.

[shellscape]

OK so this is just a weird problem. That particular recipe was part of the initial batch of recipes. I've looked through the commit history and there doesn't seem to be anything related to that recipe, meaning it was probably planned (in my head) and I forgot about it, or forgot to create a new issue to track it. It also looks like the recipe is incomplete, because the client would have to be able to respond. Now, piggy-backing on the reload event would work in theory (and it might have because as of that commit hash the router / socket code was much simpler).

Essentially what it's trying to accomplish is getting a message through to the client.

So now, what we don't want to do is add a specialized socket message for that. It'll be seldom used but in specialized cases, so it's not a great candidate to etch in stone in the codebase. The code below also has explicit meaning and should only come from a compiler action:

const action = options.liveReload ? 'reload' : 'replace';

So solid attempt at resolving this, but what we really need is a means to get messages to the client, so the client WebSocket can handle messages and perform actions easily. To accomplish that easily we have one of two choices:

1. provide access to the socket via an event or property
2. trap all unknown/unhandled events on the plugin instance and send them through the socket to the client

[1] has obvious disadvantages. If we ever change the inner workings, it's a prime candidate for regressions, and it increases complexity of the code. And destroying/creating sockets posts a problem for tracking which socket the user should send on. There's also the issue of exposing a key internal object and the possibility that a user messes with it, creating a support problem.

[2] is probably the better solution, and could be considered a "convenience" action. Event handlers can be gleaned via emitter.eventNames(), and emit() can be overridden to determine if an event being emitted is unhandled. Internally emitted events will never be unhandled, and custom events by users in configs will also never be unhandled (unless by mistake), so that's a relatively safe assumption.

So I think that's the way we should go with this. If it's OK with you, I can add those changes to your PR. (I have them ready to go, just have not pushed them to your fork yet) Thoughts? @playma256 @nicksrandall

[nicksrandall]

I'm ok with your suggestions. As long as I have some mechanism to programmatically trigger a reload, I'm happy. FWIW, this is the code I have to watch php files for changes and trigger a reload:

  const servePlugin = new Serve({
    static: "dist/",
    host: "localhost",
    port: "4430",
    hmr: DEV,
    liveReload: DEV,
    compress: true,
    https: {
      key: fs.readFileSync("./certs/server.key"),
      cert: fs.readFileSync("./certs/server.crt")
    }
  });

  if (DEV) {
    const watcher = chokidar.watch("./**/*.php", {
      ignored: /node_modules/,
      alwaysStat: true,
      atomic: false,
      followSymlinks: false,
      ignoreInitial: true,
      ignorePermissionErrors: true,
      persistent: true,
      usePolling: true
    });

    servePlugin.on("listening", () => {
      watcher.on("change", () => {
        servePlugin.emit("reload", { source: "config" });
      });
    });

    servePlugin.on("close", () => watcher.close());
  }

[shellscape]

Cool, I'll get that pushed to your patch fork tonight.

[shellscape]

@playma256 @nicksrandall please review my changes to this. I think this will give users a lot more flexibility moving forward.

[matheus1lva]

That will actually create more flexibility and simplifies a lot the logic on how events are emitted!

[nicksrandall]

Changes look good to me :)

[nicksrandall]

@shellscape or @playma256 would you mind cutting a new release with this change on NPM?

[shellscape]

We always do 🍺