enconding: Add preferred option

[bjohansebas]

Use of the preferred option in encoding

There must be a backport to version 1, as it is a feature needed for expressjs/compression#194

[blakeembrey]

Honestly, I'm not super comfortable with this code as-is and would like to remove the variadic hack from the library instead of going deeper into it with options too.

[bjohansebas]

It was the way I found to avoid breaking compatibility and to be backward compatible. It took me a while to understand why it used arguments (it's the first time I see it), maybe it can be done in cleaner ways

[blakeembrey]

Ideally we get rid of that altogether, it's hard to understand, hard to fix, and worse for performance. I'd like to do it in a new major version. For now, I'm going to suggest you skip accepts altogether by doing: expressjs/compression#194 (comment). I think it's a better change anyway, and I won't need to maintain multiple releases of accepts this way.

[wesleytodd]

I will push to oppose a new major at this stage. This package is passed through to express and we cannot do more majors in these without incurring an unnecessary maintenance burden until we are preparing for a v6 release.

I agree with the proposed breaking change from @blakeembrey, just not the timing.

[blakeembrey]

@wesleytodd Why can't we can do a major version? You can just upgrade in express v5 or v6 when you're comfortable with it. A backward incompatible API change in this package doesn't mean it changes the exposed behavior in express, and coupling them together means we can't make useful API changes and makes the idea of independent package redundant.

[wesleytodd]

Sorry if my comment was not clear. Yes I understand how we can do a major. My push for avoiding it is much more because of the complexity of maintaining multiple major lines over the next 2-3 years. We have created a situation for quite a few packages already where we need to backport things. This creates more work and more operational complexity across the project. There are some cases where this is clearly worth it, this is not one of them IMO.

IMO the best path forward for all these content negotiation packages is to be moved directly into node core. This means working with the other framework maintainers to agree on an api design to work toward and doing that in a silo here is, again IMO, quite undesirable. So adding to the problem by also committing to maintaining more major versions of these packages for the next 2-3 years is making the entire job harder for both us maintainers and users who need to debug their express apps. I don't think anyone benefits by taking this type of short sighted action.

[blakeembrey]

My push for avoiding it is much more because of the complexity of maintaining multiple major lines over the next 2-3 years.

This is not necessarily true. The maintenance schedule is the same either way. If a new major is released of this library, and it's functionally backward compatible (but backward incompatible because of changes the API), express can pull this into express 4/5 immediately. That would be functionally the same as waiting 3 years and trying to coordinate with the next major release of express.

In fact, it's more desirable, because we can upgrade express 4/5 together, leading to less people overall using the old version and minimizing the need to back port anything.

[wesleytodd]

Yeah, that's a valid point. What do you think about my goal of landing this stuff in node core? That is a large part of why I am resistant to larger breaking changes in these packages because I think there will be unnecessary churn for the ecosystem if the ultimate plan is to land a common api in core. If it weren't for that I probably would be much more on board with large refactors of these packages.

[bjohansebas]

It would really be great to see this in Node.js core, but it would probably take time to reach an agreement with the other frameworks. Meanwhile, doing it in this package might make more sense. Probably the best place for it would be nodejs/web-server-frameworks, but I see that it’s currently inactive.

[blakeembrey]

What do you think about my goal of landing this stuff in node core?

I think it's a worthwhile goal, but at the same time my goal for a major would include making these interfaces more generic (e.g. #30, #14) which is tangental to being in node core. Plus finalizing the API in way we're happy with (removing variadics to enable new options easily) could be beneficial to any node core effort as it'd be a stronger starting point to say "this API is already used in the world" vs trying to design a new unused API.