jquery: emit warning or error for .done() and .fail()

NovemLinguae commented 7 months ago

Using jQuery's .done() or .fail() instead of native JS promises .then() or catch():

1) contradicts the advice at https://www.mediawiki.org/wiki/Manual:Coding_conventions/JavaScript#Asynchronous_code

2) led to some nasty bugs in one repo I was working with, for example, this patch

I'd suggest adding something like the below to our jQuery rules. Code below credit Kosta Harlan.

"no-restricted-syntax": [
    "error",
    {
        "message": "Using .done() is not allowed. See https://www.mediawiki.org/wiki/Manual:Coding_conventions/JavaScript#Asynchronous_code",
        "selector": "MemberExpression > Identifier[name=\"done\"]"
    },
    {
        "message": "Using .fail() is not allowed. See https://www.mediawiki.org/wiki/Manual:Coding_conventions/JavaScript#Asynchronous_code",
        "selector": "MemberExpression > Identifier[name=\"fail\"]"
    }
],

jdforrester commented 7 months ago

Should this instead be a new https://github.com/wikimedia/eslint-plugin-no-jquery rule? @edg2s, thoughts?

edg2s commented 7 months ago

It would probably make sense to add the rules to the plugin, although they will be quite distinct from other rules, in that the apply to neither the $ global object, or a $collection. That said "done" and "fail" are quite uncommon names for methods outside of promises.

NovemLinguae commented 2 weeks ago

I cut and pasted this ticket over to https://github.com/wikimedia/eslint-plugin-no-jquery/issues/332. Closing in this repo, leaving open in that repo.

wikimedia / eslint-config-wikimedia

jquery: emit warning or error for .done() and .fail() #564