Rename 'filter' to 'quality'

[zbjornson]

I think this name makes more sense than "filter" anyway.

Ref #1063 and #1281 (comment)

Thanks for contributing!

• Have you updated CHANGELOG.md?

[LinusU]

Since I've already tagged 2.0.0 (sorry about that 🙈), would you mind adding a getter/setter with the old name (perhaps logging a deprecation warning)?

edit: deprecation helper -> https://nodejs.org/api/util.html#util_util_deprecate_fn_msg_code

[zbjornson]

Hrmmm that sort of defeats the purpose, I think? Could we unpublish 2.0.0 and release it as 2.0.1?

(looks like we only have 72 hours from publish date to unpublish btw)

[zbjornson]

This could also be deferred to 3.x though. Just nice to get as many breaking changes out of the way as possible. (I wonder if anyone actually uses this property?)

[LinusU]

Hrmmm that sort of defeats the purpose

I think it would be quite nice? Without a deprecation notice for the old setter, code setting the old property will not generate any warning/error (since you can add any property to any object in JS). So I would actually like it if 2.x had a deprecation warning, and 3.x throws an error when trying to set it. Then in 4.x we could remove it completely.

My mind is always open though, so please let me know if you feel otherwise ☺️

[asturur]

i do not think anyone is gonna rush to implement ctx.filter ( i would to, but is not my priority ).
It would have been nice to have road unblocked.
Please consider that if jumping to 3.0 means killing node6 while is LTS, that could be less nice.
(since the wasm changes, are targeted for next major bump and required node 8 if i understood correctly)

[zbjornson]

jumping to 3.0

Sorry, I didn't mean that we had to jump to 3.x -- just that it's not critical to have this PR landed in 2.x. (I've also been using "3.x" in a few comments when I should have said "some major release after 2.x.")

---

So I would actually like it if 2.x had a deprecation warning, and 3.x throws an error when trying to set it. Then in 4.x we could remove it completely.

Given the goal to open up that property for the implementation of the standard filter property, the two possible scenarios are these, right?

1. 2.x adds warning, 3.x gets standard impl. Downside is if someone is running isomorphic (browser+node) code that uses the standard filter attribute, it gets noisy in node instead of just being a noop (as it is in browsers that don't yet support filter).
2. 2.x removes attribute, 2.y (minor rel) gets standard impl. Until 2.y, acts the same as the browsers that don't support the property yet.

I agree a message would be friendly to users, but I don't think it's necessary (we aren't doing it for the changed globalCompositeOperators that were renamed, either). Throwing an error would be unusual and would make isomorphic code a pain.

I think this is a very low-impact PR, so I'm fine with any resolution. 🙂 My pref would still be for [semver major after 2.x] or [unpublish 2.0.0 and release 2.0.1], followed by [an alias with no warning].

[LinusU]

Ahhhhaaaa, now I see. I thought that our implementation was compatible, just named something else 😄

Hmmmm, okay. I'll unpublish 2.0.0, merge this, and release 2.0.1...

[zbjornson]

Thanks! Sorry for the hassle.

[LinusU]

No problem at all, it was I who was a bit fast to publish 2.0 😆