Better server watchers - Githubissues

jesseditson commented 7 years ago

Start Date: 2016-11-23 RFC PR: (leave this empty) Ember CLI Issue: (leave this empty)

Summary

Add the ability for ember serve and ember test --server to watch additional files that are not necessarily parsed by broccoli, but potentially used in the build. Originally requested here: https://github.com/ember-cli/rfcs/issues/25, but not fully flushed out. Primary use would be to allow files in server and test to trigger full restart, and trigger either a rebuild or provide a hook to re-run node tests when these files change.

Motivation

I use the server directory in both dev and prod (via a prod express server located in the root), and as such, have some application-server logic that I test with tests in the test folder.

As per detailed in https://github.com/kategengler/ember-cli-code-coverage/pull/79, I run pre and post build tests & coverage for my server code when running tests, but according to https://github.com/ember-cli/ember-cli/issues/4717, currently ember-cli does not support adding arbitrary paths to the watcher, which means that when I run ember test --server, the tests will not re-run when editing server tests or files.

In addition, I've noticed some idiosyncrasies when using the server folder as a first-class citizen in this build setup, namely that the server does not perform a full restart when changing files (which can lead to cached server modules). This may be an unrelated RFC candidate, but it seems like they are in the same ballpark so may be worth looking at during this work.

Detailed design

The following changes would be required:

Add a config option and/or sensible defaults for the above setup (application server files in server, application server tests in test)
Perform a full restart of the application server when changes are detected on server files

At it's most basic, this feature would simply add a new config key to .ember-cli, that would enable rebuild/restart on paths outside of the broccoli tree.

At it's most complicated, this feature would modify ember-cli to:

Allow inclusion of files outside of the broccoli tree to the watchers
Restart the node server whenever rebuilding the app

How We Teach This

Ember already supports a fairly robust application server setup, which I've been working on taking advantage of for automatic API generation over at https://github.com/jesseditson/ember-jsonapi.

This change would primarily be a functional change, but would also open up the possibility of bringing the server folder in to the docs a bit more, primarily around how to properly test the files in that folder, and clarification around expected rebuild functionality for files outside of the broccoli tree.

I believe this RFC is in the spirit of existing ember-cli conventions, and as such proposes no changes to terminology or patterns, save for potentially defining a standard path to testing server code.

Drawbacks

This could potentially broaden the scope of ember-cli, which seems worth taking very seriously from a support perspective. The server folder seems like an afterthought, and this could potentially legitimize it's use, which may not be in the interests of the maintainers.

Alternatives

As far as I can tell, this isn't currently directly achievable using the ember-cli watchers as they are set up today, otherwise I'd be exploring an addon solution for this.

Currently I solve for half of this issue this by running a docker-compose setup which runs a proxy back to my host machine's port 4200 and maintains a separate watcher process for server code. This means the server could get out of sync when restarting (as changing shared files would cause both a server and ember restart/rebuild), so I put a default nginx page that polls the host machine for the ember server, and reloads when it comes online. I think this could be greatly simplified by giving the ember server folder some love.

I don't currently have a solution for reloading the server tests when they are modified (or when the server files are modified), but I suspect a workaround would require adding an additional watcher process (I could see something like parallelizing a subprocess running watchman using an addon hook, but this has plenty of drawbacks).

Unresolved questions

Mainly I'd like to get the temperature of the room on the intended use of server. Am I opening a pandora's box of support issues if ember starts supporting an application server? On the flip side, I think minimal support on ember-cli's part would open up a good ecosystem of addons that leverage the node layer, which would allow us to share config across the client and application server (this has been very useful for me when sharing models across the stack).

Clarity necessary before action: Is this RFC best suited for a deep-dive on the server process and how it is watched, or is this RFC "simply" a modification to the available options and/or defaults of the ember-cli test watcher?

jcope2013 commented 7 years ago

would something like this work? https://github.com/ebryn/ember-autoserve

jesseditson commented 7 years ago

@jcope2013 Thanks for the link! autoserve is intended to solve for Restart the node server whenever rebuilding the app, which is more of a side-effect than the intent of this RFC.

It also uses nodemon, so it's not dissimilar from just running nodemon ember serve - useful, but won't solve for server test watching, and is pretty poor from a performance standpoint since it'll run both watchman and nodemon in parallel.

I'd much rather see a solution that properly busted the require caches or piggy-backed on watchman's existing watchers for the restart functionality. My laptop fan will thank me ;)

stefanpenner commented 7 years ago

can you post this as a PR, like this one that way line-comments can be made.

Also note, detailed design should include public API designs, and thoughts on how it can be implemented.

jesseditson commented 7 years ago

@stefanpenner will do!

stefanpenner commented 7 years ago

closing in-favor of PR. (so we can provide feedback and iterate)

ascot21 commented 7 years ago

@jesseditson did you ever submit a PR for this? I have a similar need for this functionality and was wondering if anything ever came of it.

jesseditson commented 7 years ago

Hey! We've implemented a workaround in our codebase but haven't worked it in to an open source solution yet, so no movement on this issue for now!

ascot21 commented 7 years ago

@jesseditson thanks for letting me know. I'm currently working on a workaround with a custom script and watchman

mangatinanda commented 7 years ago

@jesseditson May I know what workaround you have done?

In my use case, I have a folder(say app1) outside the app , which has its own broccoli build process(I used this to watch and rebuild) and outputs something which is used inside the ember app. I want ember server to watch app1 and reload whenever something changes inside app1.

As a workaround, I gave output path of app1 compiled output as /public. Since, ember-cli watches public and copies everything to dist and reloads, this works for me but doesn't look good.

Anybody help?

jesseditson commented 7 years ago

@mangatinanda - I can't post the full code, but here's a summary:

We use a makefile with the following task:

dev:
    ./node_modules/.bin/supervisor \
    -i .git,node_modules,dist,app,bower_components,tmp,mirage,coverage,tests \
    -e js,json \
    --debug \
    index.js

If you've done npm install --save-dev supervisor, this will watch all the non-ember files and reload the server.

In our setup we use docker-compose and proxies to deal with our server, so YMMV but index.js is an express server that mounts the ember server like this:

const express = require('express');

var app = express();

/**
 * Custom routes & proxies
 */

/**
 * Mount application level routes
 */
require('./server')(app);

// if called directly, just serve.
if (require.main === module) app.serve();

Note: our setup uses a custom express server, not the default one - so the above is untested, but should help get you on the right path

server/index.js mounts a dev server (dev only, in prod it just serves static content from dist) that proxies to ember's server (again, not the actual code we use but very similar):

var httpProxy = require('http-proxy')
var fs = require('fs')
var path = require('path')
var debug = require('debug')('proxy')
var express = require('express')
var livereloadServer = express()
var emberConfigFile = fs.readFileSync(path.join(process.cwd(), '.ember-cli'), 'utf8')
var emberConfig = JSON.parse(require('strip-json-comments')(emberConfigFile))

const LIVERELOAD_PORT = emberConfig['live-reload-port'] || 49152
const SCHEMES = { 'web': 'http://', 'ws': 'ws://' }
const SERVER_PORT = emberConfig['port'] || 4200
const NODE_HOST = process.env.DEV_HOST || '127.0.0.1'

function createProxy(host, port, type, rootURL) {
  var targetURL = `${SCHEMES[type]}${host}:${port}`
  if (rootURL) targetURL += `/${rootURL}`
  var proxy = httpProxy.createProxyServer({
    changeOrigin: true,
    ws: type === 'ws',
    target: targetURL
  })
  return function(req) {
    var args = Array.prototype.slice.call(arguments)
    var fromURL = req.originalUrl || 'socket'
    debug(`Proxying ${type} ${fromURL} -> ${targetURL}`)
    var errHandler = err => {
      var errorMessage = `Error proxying ${fromURL} -> ${targetURL}: ${err.message}`
      console.error(errorMessage, err.stack)
    }
    args.push(errHandler)
    proxy[type].apply(proxy, args)
  }
}

module.exports = function(app, config) {
  config = config || {}
  var devProxy = createProxy(NODE_HOST, SERVER_PORT, 'web', config.rootURL)
  var lrWsProxy = createProxy(NODE_HOST, LIVERELOAD_PORT, 'ws', config.rootURL)
  var lrWebProxy = createProxy(NODE_HOST, LIVERELOAD_PORT, 'web', config.rootURL)

  // in dev, proxy to the ember server running on DOCKER_HOST
  app.use(devProxy)
  // also create a proxy server for livereload
  livereloadServer.serve = function() {
    this.set('port', this.get('port') || LIVERELOAD_PORT)
    this.set('host', this.get('host') || process.env.LIVERELOAD_HOST || '0.0.0.0')
    var server = require('http').createServer(this)
    server.on('upgrade', lrWsProxy)
    server.listen(this.get('port'), this.get('host'), function() {
      debug(`Livereload proxy server running on port ${server.address().port}`)
    })
  }
  livereloadServer.use('/livereload', lrWsProxy)
  livereloadServer.use(lrWebProxy)
  livereloadServer.serve()
}

Once this is set up, you'll need to run both ember serve and make dev. We have build scripts that handle the process management for us, but you could achieve something similar by using bash and exit traps (once again, just writing this from memory, so could need some tweaking):

dev.sh

#!/bin/bash -e

function stop {
  kill $(cat tmp/ember.pid)
}
trap stop EXIT

mkdir -p tmp
ember serve  &> tmp/ember.log &
echo $! > tmp/ember.pid
make dev

To run the above, just chmod +x dev.sh and run directly (./dev.sh), or run it from npm scripts:

...
  "scripts": {
    "dev": "./dev.sh"
  }
...

then you can npm run dev and get your whole stack, and in theory it should all stop when you CTRL-C your forgeground ember server.

Logs are another issue, to get your full logs you'll need both the foreground server logs and the background ember logs - you could open a new tab and tail ember.log for the ember logs which is likely the simplest solution - or you could use a wrapper script to handle your processes (which I won't go in to here) that replaces the dev.sh script above with something more robust and monitorable. It's possible that pm2 would be up to the task.

I realize this isn't the most precise of a playbook, but hopefully it'll give you some good ideas to work with.

ember-cli / rfcs

Better server watchers #81

Summary

Motivation

Detailed design

How We Teach This

Drawbacks

Alternatives

Unresolved questions