paulmillr / pushserve

Dead-simple pushState-enabled command-line http server.
32 stars 8 forks source link

[enhancement] emulate loading? #3

Closed KATT closed 10 years ago

KATT commented 10 years ago

Hey,

Just wanna see if you think this is a good idea or not and something you'd consider if I'd make a pull request.

I've been using brunch for for some prototypes and stuff. One thing that's always difficult to test locally is loading times on asynchronous stuff or loading state when loading images on the site etcetera. I can do this by adding setTimeouts here and there, but it's ugly and variables like emulateLoading and junk tends to end up in the application.

Would be schweet if I could add some random delay when serving assets and AJAX requests ( request.headers['x-requested-with'] == 'XMLHttpRequest').

What do you think?

paulmillr commented 10 years ago

not interested, sorry.

i suggest making your own fork

KATT commented 10 years ago

Hm. How do I go abouts overriding brunch's pushserve with my own fork?

paulmillr commented 10 years ago

You can use custom server for brunch w -s.

https://github.com/brunch/brunch/blob/stable/docs/config.md#server

KATT commented 10 years ago

Ah, thanks!

KATT commented 10 years ago

However if you do this you won't get all the options from the config. Brunch just gives it port, publicPath and log.

From watch.coffee:

  # [..]
  if config.server.path
    try
      server = require sysPath.resolve config.server.path
    catch error
      logger.error "couldn\'t load server #{config.server.path}: #{error}"
    unless server.startServer?
      throw new Error 'Brunch server file needs to have startServer function'
    server.startServer port, publicPath, log
  else
    opts = noLog: yes, path: publicPath
    pushserve helpers.extend(opts, serverOpts), log

Why not instead assuming it would has the same interface as pushserve and serve it the same options?

I.e.

  # [..]
  opts = noLog: yes, path: publicPath
  opts = helpers.extend(opts, serverOpts)
  if config.server.path
    try
      server = require sysPath.resolve config.server.path
    catch error
      logger.error "Couldn't load server #{config.server.path}: #{error}"

    try
      server opts
    catch error
      throw new Error "Brunch server needs be a function"
  else
    pushserve opts, log

Makes your own servers more extensible.

Yes/no? Could do PR on brunch.

paulmillr commented 10 years ago

You can load config.coffee from server.coffee. There's no need to pass it around.

KATT commented 10 years ago

Dependency injection is cleaner + that's how it works with pushserve already, why use two different patterns?

paulmillr commented 10 years ago

Backwards-compatibility. We can't break existing servers / apis.

KATT commented 10 years ago

I'll make a PR with backwards-compability on startServer and (if you want) add a deprecation warning on startServer so you can remove that later.

Was gonna do a PR this morning but couldn't build brunch. I'll have another go later.

✪ npm test

> brunch@1.7.13 test ~/dev/brunch
> node setup.js test

Executing node_modules/.bin/mocha --compilers coffee:coffee-script --require test/common.coffee --colors

~/dev/brunch/test/fs_utils.common.coffee:1
exports, require, module, __filename, __dirname) { common = require '../src/fs
                                                                    ^^^^^^^^^^^^^^^^^^^^^^^^
SyntaxError: Unexpected string
  at Module._compile (module.js:439:25)
  at Object.Module._extensions..js (module.js:474:10)
  at Module.load (module.js:356:32)
  at Function.Module._load (module.js:312:12)
  at Module.require (module.js:364:17)
  at require (module.js:380:17)
  at ~/dev/brunch/node_modules/mocha/lib/mocha.js:152:27
  at Array.forEach (native)
  at Mocha.loadFiles (~/dev/brunch/node_modules/mocha/lib/mocha.js:149:14)
  at Mocha.run (~/dev/brunch/node_modules/mocha/lib/mocha.js:306:31)
  at Object.<anonymous> (~/dev/brunch/node_modules/mocha/bin/_mocha:343:7)
  at Module._compile (module.js:456:26)
  at Object.Module._extensions..js (module.js:474:10)
  at Module.load (module.js:356:32)
  at Function.Module._load (module.js:312:12)
  at Function.Module.runMain (module.js:497:10)
  at startup (node.js:119:16)
  at node.js:902:3