Make static-serve inherit the "etag" application setting when used with Express

[sjanuary]

From the Express 5.0 roadmap expressjs/express#2237 and expressjs/express#2317

If the etag setting is not explicitly set for serve-static and it is being used with Express, serve-static will inherit the etag setting from Express.

I have written tests for this PR and run them, but I have written them against the Express project rather than serve-static because the scenario requires both. Tests look like this:

it('should override static-serve etag setting if not set', function (done) {
      var fixtures = __dirname + '/fixtures';
      var app = express();

      app.disable('etag');
      app.use(express.static(fixtures));

      request(app)
      .get('/name.txt')
      .expect(function (res) {
        assert.ok(!('etag' in res.headers), 'should not have etag header')
      })
      .expect(200);

      app.set('etag', true)

      request(app)
      .get('/name.txt')
      .expect(function (res) {
        assert.ok(('etag' in res.headers), 'should have etag header')
      })
      .expect(200, done);
    })

    it('should not override static-serve etag setting if set', function (done) {
      var fixtures = __dirname + '/fixtures';
      var app = express();

      app.disable('etag');
      app.use(express.static(fixtures, {'etag' : true}));

      request(app)
      .get('/name.txt')
      .expect(function (res) {
        assert.ok(('etag' in res.headers), 'should not have etag header')
      })
      .expect(200);

      app.set('etag', true)

      request(app)
      .get('/name.txt')
      .expect(function (res) {
        assert.ok(('etag' in res.headers), 'should have etag header')
      })
      .expect(200, done);
    })

    it('should not override static-serve etag setting if disabled', function (done) {
      var fixtures = __dirname + '/fixtures';
      var app = express();

      app.disable('etag');
      app.use(express.static(fixtures, {'etag' : false}));

      request(app)
      .get('/name.txt')
      .expect(function (res) {
        assert.ok(!('etag' in res.headers), 'should not have etag header')
      })
      .expect(200);

      app.set('etag', true)

      request(app)
      .get('/name.txt')
      .expect(function (res) {
        assert.ok(!('etag' in res.headers), 'should have etag header')
      })
      .expect(200, done);
    })

[dougwilson]

Middleware are self-contained. This change needs to be done in Express, not this module.

[sjanuary]

@dougwilson ok, I understand the principle that serve-static shouldn't know about Express, but it doesn't seem like there's any way to change the options after staticServe() has been called. Would either a PR that looks for the etag setting on the request object or a PR that allows the options to be changed be more likely to be accepted in this module? I don't think it's possible to do this entirely in Express.

[dougwilson]

No PR for this would be accepted here, as even your suggestion wouldn't work, because of race conditions.

If you're not sure how to approach this change, you may want to discuss in our Gitter room or over in Express to better understand the requirements or how to approach the problem before tackling it. Currently I have a backlog of PRs to review, so will not have time to personally discuss this for a few weeks.

[sjanuary]

Ok, thanks for the feedback