goodeggs / teacup

Teacup is templates in CoffeeScript
MIT License
122 stars 18 forks source link

Support of hot templates reconnection #28

Open isglazunov opened 11 years ago

isglazunov commented 11 years ago

Tracking changes to the template and its reconnection if changed Maybe I did not understand something and it's already implemented?

mark-hahn commented 11 years ago

reconnection if changed

Teacup doesn't connect to anything. It is just a function that creates HTML text.

On Sun, Sep 22, 2013 at 5:34 AM, isglazunov notifications@github.comwrote:

Tracking changes to the template and its reconnection if changed Maybe I did not understand something and it's already implemented?

— Reply to this email directly or view it on GitHubhttps://github.com/goodeggs/teacup/issues/28 .

isglazunov commented 11 years ago

Thank you. Probably my fault. However, I considered teacup / lib / express as a solution to express framework. It is to this part of the code was my question. Perhaps we should make the change require, which is specifically for this case will not reproduce the template code with require, and eval or new process.

I apologize for my English of Google translate.

Or should make a connection to express in a separate module.

PS TeaCup - genius and simple solution. I very much want to do it / him for one chip, but prevents a cold connection files.

hurrymaplelad commented 11 years ago

@isglazunov teacup doesn't support the cache: false option for express template engines that would let you re-render templates without restarting express.

I spent a little time thinking about what support would take a while back. The Consolidate.js Teacup adapter has a half-baked implementation. The two big problems I remember were:

  1. Reloading transitive requires. If a template requires some shared helpers and the shared helpers change, the shared helpers should be dumped from the require cache. Some modules depend on the require cache for shared state. Seems doomed to have flaky edge cases.
  2. The consolidate.js eval based adapter chokes on requires with relative paths (like require('./foo')) in the tempates.

If you find a solution that feel general purpose and robust, I'd love to pull it into teacup. A less general solution (like only supporting templates without require statements) could also be useful, and probably belongs in a separate module.

isglazunov commented 11 years ago

I chenge file teacup/lib/express.js

// Generated by CoffeeScript 1.6.3
(function() {
  var CoffeeScript;

  CoffeeScript = require('coffee-script');

  module.exports = {
    renderFile: function(path, options, callback) {
      var err;
      try {
        console.log(options.app); // options.app is undefined in express 3.4.0
        if (!options.cache) { // but options.cache is boolean
            delete require.cache[require.resolve(path)];
        }
        return callback(null, require(path)(options));
      } catch (_error) {
        err = _error;
        return callback(err);
      }
    }
  };

}).call(this);

It work!

isglazunov commented 11 years ago

I suggest to change the strategy. Make a Teacup constructor having an argument "cache".

Teacup = require ("teacup")
teacup = Teacup cache: false

Make teacup.require depending on the argument cache. Make it something like Teacup.Express as an alternative constructor that could pass as a engine.

isglazunov commented 11 years ago

If you want, here is a test room. https://github.com/isglazunov/teacup.rerendering.test

hurrymaplelad commented 11 years ago

The problem I anticipate looks like:

server.coffee:

#...
server.get "/", (request, response) ->
    response.render "template.coffee"

template.coffee:

{renderable} = require 'teacup'
{contactUser} = require './helpers.coffee'

module.exports = renderable (users) ->
  for user in users
    contactUser(user)

helpers.coffee:

{p, renderable} = require 'teacup'

module.exports = renderable ({name, email}) ->
  p "Contact #{name} at #{email}"

How do I make sure '/' updates when I make changes to helpers.coffee?

This might be easier to communicate with some automated tests and a pull request.

isglazunov commented 11 years ago

Earlier I wrote about the teacup.require. It can be used to connect patterns. Not a very nice wrapper, but either he or a third-party plug-ins.

scien commented 10 years ago

looks like this was already implemented? This issue can probably be closed, right?

module.exports =
  renderFile: (path, options, callback) ->
    try
      # If express app does not have view cache enabled, clear the require cache
      if options.app? and not options.app.enabled('view cache')
        delete require.cache[require.resolve path]
      callback null, require(path)(options)
    catch err
      callback err
hurrymaplelad commented 10 years ago

I added a failing test on the branch bust-cache to illustrate the case described above where the implemented solution breaks down.

Feels error prone to me, so I've left the issue open, but totally depends how you use Teacup. If you don't require template helpers, then yes, the issue is solved for you. With templates split across files I still prefer more general solutions like nodemon or node-dev.

anodynos commented 10 years ago

Perhaps you should have a look at http://stackoverflow.com/questions/9210542/node-js-require-cache-possible-to-invalidate - I adapted the code from one of the answers and published it as 'require-clean' - it seems to be working for uRequire's support for Teacup refreshing templates.

hurrymaplelad commented 10 years ago

Thanks for the heads up @anodynos. I hadn't seen require.cache[module].parent. Any idea if we can get all the parents (requirers) of a module?

In the example above, if two templates used helpers.coffee, we'd want to dump both from the cache when helpers.coffee changes, but it looks like a module only keeps track of one parent...

anodynos commented 10 years ago

I guess you could: find recursively which modules have module A as a child, then these are A's parents. But you'll have to go through the whole cache recursively which would be slowish. Also you would also need to clean all parent's parents and so on.

But I wonder if that parent reseting is really needed. Any time module A needs to load, it needs to load its self fresh along with all its children (require('child')'s). Again all the children of each child needs to be refreshed recursively, which is what is happening within 'require-clean'.

But why do we need to refresh the parent's of A, since the parent will reset it self & children when the parent actually loads (i.e when '/' is requested ) ?

hurrymaplelad commented 9 years ago

So we'd require the template and all its helpers clean on every request, even when nothing has changed... Sounds like that'd work! I was getting hung up on change tracking but we may not need it at all. Something like:


  renderFile: (path, options, callback) ->
    try
      # If express app does not have view cache enabled, clear the require cache
      if options.app?.enabled('view cache')
        template = require path
      else
        requireClean = require 'require-clean'
        template = requireClean path
      callback null, template(options)
    catch err
      callback err