Skip to content
This repository was archived by the owner on Sep 13, 2018. It is now read-only.

Adding Support For Caching Headers (because 301 is forever) #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gja
Copy link

@gja gja commented Oct 4, 2016

No description provided.

@@ -21,6 +21,13 @@ module.exports = function (redirects, port) {
res.statusCode = redirect.code || 302;
res.setHeader('Content-Type', 'text/plain');
res.setHeader('Location', newUrl);

if(redirect.headers) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if (

@@ -21,6 +21,13 @@ module.exports = function (redirects, port) {
res.statusCode = redirect.code || 302;
res.setHeader('Content-Type', 'text/plain');
res.setHeader('Location', newUrl);

if(redirect.headers) {
for(var header in redirect.headers) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Make this the following:

Object.keys(redirect.headers).forEach(function (header) {

@pksunkara
Copy link
Contributor

@gja Are you planning to address my review comments?

@gja
Copy link
Author

gja commented Oct 29, 2016

Sorry. Will do in a day or so

Tejas Dinkar

On 29 October 2016 at 6:33:08 AM, Pavan Kumar Sunkara (
[email protected]) wrote:

@gja https://github.com/gja Are you planning to address my review
comments?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AACHumrZ8bUt1ccN2kSCcrMEvgZZP7A-ks5q4ptMgaJpZM4KODpn
.

@playground
Copy link

I can't submit an issue. Is this working for you?
var config = require("nconf")
.argv()
.file({file:__dirname+"/../config.json"});

this __dirname+"/../config.json" would produce node-redirect/bin/../config.json

@marcofugaro
Copy link

Dude wtf, you just made a plugin that redirects to your website

@pksunkara
Copy link
Contributor

@marcofugaro You do know you can configure the host, right?

@marcofugaro
Copy link

Not without touching the node_modules folder

@pksunkara
Copy link
Contributor

pksunkara commented Jan 19, 2017

@marcofugaro This is always intended to be a direct website. Not a dependency or middleware

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

Successfully merging this pull request may close these issues.

4 participants