search

balderdashy / sails

Realtime MVC Framework for Node.js
https://sailsjs.com
MIT License
22.85k stars 1.95k forks source link

Handlebars: the ejs layout is not been cached. #4301

Open dc-me opened 6 years ago

dc-me commented 6 years ago

Sails version:latest Node version:v8.9.4 NPM version:v5.6.0 DB adapter name: N/A DB adapter version: N/A Operating system:N/A

the layout feature in default-view-rendering-fn.js is not correctly set cache, the ejs doc says: 1.cache Compiled functions are cached, requires filename 2.filename The name of the file being rendered. Not required if you are using renderFile(). Used by cache to key caches, and for includes.

image which is incorrect, should not delete the options.filename, and set options.cache = true I've test this and it works, the layout should be cached as it's content is not change much, makes it renders faster!

sailsbot commented 6 years ago

@danywheeler Thanks for posting, we'll take a look as soon as possible.

For help with questions about Sails, click here. If you’re interested in hiring @sailsbot and her minions in Austin, click here.

mikermcneil commented 6 years ago

@danywheeler Do you have benchmarks handy-- even if it's just happenstance response time numbers from your app, any kind of relevant before vs. after metric would be helpful as far as prioritizing this. Thanks for looking into it!

dc-me commented 6 years ago

well I don't have any benchmarks, I use handlebars for view, but in prod env I must restart the app for the changed layout to take effect, so I generate a new sails app, run some test, I works without reload the app, and I dig into the source code, and found out that the layout is not cached, so it didn't need reload, and also the doc should list the block usage, if I don't dig source code, I don't even know that I can do <% stylesheet('app.css') %> and then layout <%- stylesheets %> or <%- blocks.stylesheets %> to inject block code.

dc-me commented 6 years ago

and finally i think the block function in default-view-rendering-fn.js should take a third parameter with a mode , it's value is one of prepend、append、replace for where to put that html. @mikermcneil

dc-me commented 6 years ago

I'd like also to point out, if in production use other view engine like handlebars, do I need to wait for the grunt task to complete to register the partials? or how to handle the layout change then update it's cache ?

mikermcneil commented 6 years ago

Thanks for the thoughtful responses, @danywheeler. Unfortunately, it's hard for me to dig into this without benchmarks demonstrating how it's affecting Sails users with the default setup, and since we're not using hbs on any customer projects right now