JonathanBennett / prerender-redis-cache

MIT License
32 stars 30 forks source link

Rename plugin methods according to new Prerender methods. #12

Closed dzydzany closed 6 years ago

dzydzany commented 6 years ago

Prerender changed plugin methods, so here are the new ones.

thoop commented 6 years ago

Make sure this gets a major semver bump since we did a major semver bump on Prerender too, so this will require an upgrade of Prerender. Thanks for your work on this repo!

dzydzany commented 6 years ago

I am actually thinking that leaving both methods would do no harm, as nothing happens when you try to use it with different method. That way it could be compatible with old and new versions. Let me know your thoughts. Prerender master branch is already using new ones.

hadifarnoud commented 6 years ago

I don't think master branch is using new methods. it does not work with new prerender @dzydzany @JonathanBennett

hadifarnoud commented 6 years ago

I used your forked version @dzydzany and still can't see any cached page in Redis KEYS * returns empty list.

I'm on Heroku with REDIS_URL set

#!/usr/bin/env node
var prerender = require('./lib');

var server = prerender({
    chromeLocation: '/app/.apt/usr/bin/google-chrome'
});
server.use(require('prerender-redis-cache'));
server.use(prerender.sendPrerenderHeader());
// server.use(prerender.blockResources());
server.use(prerender.removeScriptTags());
server.use(prerender.httpHeaders());
// server.use(prerender.inMemoryHtmlCache());
// server.use(prerender.s3HtmlCache());

server.start();

am I doing anything wrong?

hadifarnoud commented 6 years ago

@dzydzany I get is error: Redis Cache Error: Error: Auth error: ERR unknown command 'auth'

14:13:01 0|server   |      '--hide-scrollbars' ] }
14:13:01 1|server   | 2017-12-05T14:13:01.154Z Starting Prerender
14:13:01 1|server   | 2017-12-05T14:13:01.155Z Starting Chrome
14:13:01 0|server   | 2017-12-05T14:13:01.155Z Starting Prerender
14:13:01 0|server   | 2017-12-05T14:13:01.155Z Starting Chrome
14:13:01 0|server   | 2017-12-05T14:13:01.244Z Prerender server accepting requests on port 3000
14:13:01 1|server   | 2017-12-05T14:13:01.244Z Prerender server accepting requests on port 3000
14:13:01 1|server   | Redis Cache Error: Error: Auth error: ERR unknown command 'auth'
14:13:01 0|server   | Redis Cache Error: Error: Auth error: ERR unknown command 'auth'
14:13:01 0|server   | 2017-12-05T14:13:01.717Z retrying connection to Chrome...
14:13:01 1|server   | 2017-12-05T14:13:01.718Z retrying connection to Chrome...
14:13:02 1|server   | 2017-12-05T14:13:02.722Z retrying connection to Chrome...
14:13:02 0|server   | 2017-12-05T14:13:02.725Z retrying connection to Chrome...
14:13:03 1|server   | 2017-12-05T14:13:03.725Z retrying connection to Chrome...
14:13:03 0|server   | 2017-12-05T14:13:03.727Z retrying connection to Chrome...
14:13:04 1|server   | 2017-12-05T14:13:04.740Z Started Chrome: HeadlessChrome/62.0.3202.94
14:13:04 0|server | 2017-12-05T14:13:04.752Z Started Chrome: HeadlessChrome/62.0.3202.94
jeerbl commented 6 years ago

Hi @dzydzany !

Your PR is not complete. In fact, the hook names changed but there are other changes in the prerender middleware. req.prerender.documentHTML do not contain the prerendered document, instead, req.prerender.content does.

You should add a check in the pageLoaded hook and replace the first line of this function with:

if (redis_online !== true || !req.prerender.content) {

And also the client.set line with:

client.set(req.prerender.url, req.prerender.content.toString(), function (err, reply) {

Please make the updates before this can be merged by @JonathanBennett.

Thanks Jerome

JonathanBennett commented 6 years ago

Hi All. Just wanted to say sorry for the silence - had notifications turned off and have been all over the place. Have published version 0.2.0 which is compatible with the newest version of prerender. Give it a try. 🎉 Thanks for your continued use!