Accidental breaking change on v15.7.0

[mmarchini]

• Version: v15.7.0
• Platform: OS X (should be reproducible on any OS)
• Subsystem: http2

What steps will reproduce the bug?

This issue was introduced on #36505

Try to assign something to req on an instance of Http2ServerResponse.
Example error: https://github.com/restify/node-restify/runs/2081960519?check_suite_focus=true#step:5:242

TypeError: Cannot set property req of [object Object] which has only a getter
    at Server._setupRequest (/home/runner/work/node-restify/node-restify/lib/server.js:1282:13)
    at Server._onRequest (/home/runner/work/node-restify/node-restify/lib/server.js:967:10)
    at Http2SecureServer.emit (node:events:378:20)
    at Http2SecureServer.EventEmitter.emit (node:domain:532:15)
    at Http2SecureServer.onServerStream (node:internal/http2/compat:870:10)
    at Http2SecureServer.emit (node:events:378:20)
    at Http2SecureServer.EventEmitter.emit (node:domain:532:15)
    at ServerHttp2Session.sessionOnStream (node:internal/http2/core:2930:19)
    at ServerHttp2Session.emit (node:events:378:20)
    at ServerHttp2Session.EventEmitter.emit (node:domain:532:15)

Example code: https://github.com/restify/node-restify/blob/master/lib/server.js#L1282

How often does it reproduce? Is there a required condition?

Every time

What is the expected behavior?

The code should not break on a semver-minor

What do you see instead?

An error is thrown

Additional information

This happens because getters are not assignable on strict mode. We can't revert the change because that would also be a breaking change, so we have two options:

• Change it from a getter to a normal property, so that user code doesn't break. We can go back to a getter on v16
• Leave as is, assuming that users shouldn't be assigning to the response object anyway and because v15 is not an LTS version, and then add a note to the changelogs that this was an accidental breaking change that won't be reverted

If we go with the latter we might want to discuss making all properties on req/res non-assignable (which IMO is not desirable).

cc @nodejs/tsc @nodejs/http2 @nodejs/releasers

[ghermeto]

cc/ @nodejs/web-server-frameworks

[targos]

@mmarchini do you think this is just an unfortunate breakage because due the popularity of theres.req = req pattern?
Should we start to treat every addition of a non-writable property to a public object as semver-major?

[mcollina]

Should we start to treat every addition of a non-writable property to a public object as semver-major?

I think so, yes. It's very easy to support backward compatible behavior anyway.

[mmarchini]

I agree with @mcollina, it's easy enough to ensure backward compatible behavior that we should avoid breaking it whenever possible.

[Ethan-Arrowood]

I'm +1 for treating it as semver major since if someone is currently assigning to the property, making it readonly will break their code