Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support of hot templates reconnection #28

Open
isglazunov opened this issue Sep 22, 2013 · 14 comments
Open

Support of hot templates reconnection #28

isglazunov opened this issue Sep 22, 2013 · 14 comments

Comments

@isglazunov
Copy link

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

@mark-hahn
Copy link

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//issues/28
.

@isglazunov
Copy link
Author

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
Copy link
Contributor

@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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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

@hurrymaplelad
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

scien commented May 25, 2014

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants