ember fastboot --watch

[pwfisher]

@rwjblue Tests coming soon, just want to open the PR for some early feedback. Does this approach look reasonable?

[danmcclain]

I'd argue that ember fastboot should watch by default like ember serve does

[pwfisher]

I'm thinking SIGHUP would be more appropriate than SIGUSR1.

For this reason, applications that did not require a controlling terminal, such as daemons, would re-purpose SIGHUP as a signal to re-read configuration files, or reinitialize. This convention survives to this day in packages such as Apache and Sendmail. [Wikipedia]

[jasonmit]

I recall some issues in the past where programmatically restarting a server required I also flush require.cache otherwise stale modules were loaded. I could be wrong, just jotting down something that came to mind.

Likely only a cause for concern if we plan to support adding custom middleware.

[jasonmit]

Ah, now I remember where this was an issue: https://github.com/ember-cli/ember-cli/blob/da4749d3b27a2ce7c529de0d07c0ac3e702e7d47/lib/tasks/server/express-server.js#L238-L250

[pwfisher]

Thanks, I'm adding that.

[pwfisher]

The issue would affect dist/fastboot/node_modules, which we require in buildWhitelistedRequire

[jasonmit]

👍

[leo]

@danmcclain Great idea! And it should be disabled if the --environment flag is set to "production".

[stefanpenner]

I would really like it if fastboot and normal serve could be united during development. An early discussion I started about making app.js the same regardless of fastboot or browser would nicely enable this.

[danmcclain]

@stefanpenner We actually talked about this during the last FastBoot Sync meeting

[rwjblue]

I think we should try to avoid stopping and starting the server for each reload. With some tweaks to our usage of fastboot-express-middleware we should be able to make this restartPromise closer to:

this.restartPromise = RSVP.resolve()
  .then(() => this.clearRequireCache(options.outputPath))
  .then(() => this._fastboot.reload())
  .finally(() => { /* snip from existing */ })

[pwfisher]

Faster restart will be a separate task.

[rwjblue]

I left a few inline comments, but overall this looks very good.

Also, stylistic nit RE: const: I'd prefer to stay in line with Ember and only use const in module scope.

[pwfisher]

Per discussion, we will leave the npm install on restart since it will "just work" and potential optimization will be complex (dist/node_modules is nuked by the build and the naïve solution would rely on require finding its dependencies up the directory tree—but this is not guaranteed to work). An optimized restart would be a separate PR.

const usage can land as-is and a separate PR will add and apply the linter rules from Ember.

[danmcclain]

I'm going to add homu.io use that to merge this, will do so tonight

[danmcclain]

Homu has been dead for days 😢