`prefer-dom-node-remove` bad autofix in a polyfill

[axetroy]

Autofix can cause an infinite loop when defining polyfills

All I can think of is to ignore the polyfills directory

![Element.prototype, CharacterData.prototype, DocumentType.prototype].forEach(function (item) {
    // eslint-disable-next-line no-prototype-builtins
    if (item.hasOwnProperty('remove')) {
        return
    }
    Object.defineProperty(item, 'remove', {
        configurable: true,
        enumerable: true,
        writable: true,
        value: function remove() {
-            this.parentNode.removeChild(this)
+            this.remove()
        }
    })
})

[sindresorhus]

Maybe the auto fixer could skip it if the containing method name is remove.

[fregante]

I think polyfills are exceptional. They have to do a lot of hacking things that you wouldn't do normally.

This rule specifically assumes that the environment has Element#remove so it's strictly incompatible with a Element#remove polyfill.

This is the perfect case for a eslint-disable-next-line comment

[axetroy]

This is the perfect case for a eslint-disable-next-line comment

The problem isn't whether it's disabled or not, it's that it's an unsafe autofix.

Do we have a means to detect that the current context is not suitable for autofix to reduce false positives?

For example:

// Detect the function in the current context this is assigned to Element.prototype
Element.prototype.remove = function () {
  this.parentNode.removeChild(this)
}

// Detect the function is names 'remove'
Object.defineProperty(Element.prototype, 'remove', {
    configurable: true,
    enumerable: true,
    writable: true,
    value: function remove () {
      this.parentNode.removeChild(this)
    }
})

[fregante]

All non-stylistic autofixes are unsafe if you target IE 3. If you have to cater to the lowest common denominator then disable eslint. Here are some basic examples from eslint itself:

• https://eslint.org/docs/latest/rules/no-var
• https://eslint.org/docs/latest/rules/prefer-arrow-callback
• https://eslint.org/docs/latest/rules/prefer-const

For me the linter is especially useful because it pushes me to use the best APIs possible, not the ones that are supported by the most browsers.

In this particular case, the issue is not the autofix, it’s the rule itself. The rule cannot be applied to code expected to run in an environment without Element#remove()

[fregante]

This API in particular has been available since 2013 (2015 in Edge): https://developer.mozilla.org/en-US/docs/Web/API/Element/remove

The only reason to have this polyfill is to support IE11. Doubt it’s worth discussing. If there are other exceptions unrelated to the lack of the API then a new issue can be opened. For example:

• https://github.com/sindresorhus/eslint-plugin-unicorn/issues/2372