expressjs
feat(response): new setting strict status codes
I think we need to add a test that is outside the strict range -- I could not find any existing
@wesleytodd maybe I misunderstood, could you confirm the default value of the setting?
Default: true or Default: false
I could be wrong, but I thought we had discussed with @jonchurch doing the strict checks by default in v5. I am not strongly opinionated either way, but if we dont turn it on by default now then we cannot until v6.
@wesleytodd I've updated the code according to the conversation, now "strict status codes" is true by default.
@wesleytodd, this isn't strictly required for v5
We had discussed adding this setting, and Blake suggested we err on the side of unopinionated and not set this by default. Which Ulises and myself agreed to.
It's a nice to have, but unless we open the discussion to have this throw behavior as the default, the setting itself (default off) can come in as a minor.
Ok, so how about this. To keep things moving, lets go back to not setting it by default and then we can land it either now if we get approvals, or we can release it as a minor soon after the main release.
Ok, so how about this. To keep things moving, lets go back to not setting it by default and then we can land it either now if we get approvals, or we can release it as a minor soon after the main release.
@jonchurch That's my bad, I don't think I was clear, when I wrote:
I'd love to take a strong stance either way
I meant I'd prefer no configuration. Revisiting this a few months later I actually think this is still correct, and it should be loose by default, and anything else can be middleware. My hot take here is that we should actually be deprecating and removing configuration instead of adding more.
Edit: I'm not going to block on configuration being added though, that's just my 2c on the overall subject.
I meant I'd prefer no configuration.....My hot take here is that we should actually be deprecating and removing configuration instead of adding more.
I probably agree, that said this PR is in line with historical design of the project. I am sorry for any churn I created by asking the setting to be on by default. I think we should work toward aligning on some of these "technical vision" ideas after we release v5 and work toward applying those ideals in v6 (where landing this as a minor could be a step toward that, or closed if we decide the goal is to remove configuration options).
To be clear, what folks agreed to proceed with was the validation of the range of what node accepts -- there was discussion about limiting the range further, about whether the framework should be more or less opinionated, and so on, but the idea of a strict mode limiting the range further was the last comment and did not receive further discussion AFAIU. So -- I don't think we should land this in 5.0. We could still land it as a minor if we can get consensus to land it, but I don't want to block 5.0 on this.