Skip to content

Conversation

@olivercox
Copy link

If options.settings['theme'] is set by calling

"app.set('theme', __dirname + '/themes/' + storeConfig.ActiveTheme);"

then all templates will be loaded from the themes folder if they exist.
If they don't exist templates will be loaded as usual.

If options.settings['theme'] is set by calling

"app.set('theme', __dirname + '/themes/' + storeConfig.ActiveTheme);"

then all templates will be loaded from the themes folder if they exist.
If they don't exists templates will be loaded as usual.
index.js Outdated
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the call to existsSync should be cached in production apps.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that makes sense. Not sure how to do this but will take a look.
Could the current view cache be used?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would something like this work for the current cache


var engine = options.settings['view engine'] || 'ejs'
  , desiredExt = '.' + engine
  , ext = extname(file) || desiredExt
  , key = [themeFilePath, file, ext].join('-');
//Check if the file has already been resolved in the theme folder to avoid io
if (options.cache && cache[key]) file = cache[key]; 
if (fs.existsSync(resolve(themeFilePath, basename(file)))) {
    cache[key] = resolve(themeFilePath, basename(file));
    file = resolve(themeFilePath, basename(file));
}

@RandomEtc
Copy link
Owner

This looks simple enough - can you add some tests maybe an example? Apologies up-front that the tests are a little bit of a rats' nest, feel free to start a separate file.

Added the use of the cache object to store templates that have already
been resolved in the theme folder
@bebraw
Copy link

bebraw commented Jul 4, 2013

+1

Any progress with this?

By the way as a workaround it would be awesome if relative paths worked. Seems to fail with good old ...

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

Successfully merging this pull request may close these issues.

3 participants