Atom: `addIconToElement` disposable should revert DOM modifications

[jgebhardt]

I'm looking at potentially bringing file-icons support back to Nuclide (https://github.com/facebook/nuclide/issues/951 – sorry for the long delay).

Since Nuclide uses React, DOM nodes for file names get re-used _very_ frequently, eg. in quick-open and in the file-tree. I've built a React wrapper that applies addIconToElement to the underlying DOM nodes whenever the path changes. However, the disposable returned from addIconToElement does not reset the classnames applied by addIconToElement to the DOM node. I think it would be cleaner if the disposable not only removed references to the attached node from file-icons, but actually reverted all DOM side-effects caused by addIconToElement.

console.log(element.className);
// 'initial-classname'

const disposable = addIconToElement(element, 'foo.js');
console.log(element.className);
// 'initial-classname js-icon medium-yellow'

disposable.dispose();
console.log(element.className);
// CURRENT: 'initial-classname js-icon medium-yellow'
// IDEAL: 'initial-classname'

[jgebhardt]

Awesome. Thanks for the quick turnaround!

[Alhadis]

Nope, thank YOU for taking care of this for us. Seriously.

Can you ping me once Nuclide has integrated file-icon support, please?

[jgebhardt]

@Alhadis just a heads-up, next weeks' Nuclide release will include the file-icons compatibility fixes (and it looks _really_ nice; thanks for all your work on this!)

We usually release Tuesday afternoon-ish (PST), ±24h.

[Alhadis]

Oh man, music to my ears. Thank you SO much!

I'll cut a new release right at the same time to minimise friction.

[jgebhardt]

It's out! https://github.com/facebook/nuclide/releases/tag/v0.219.0

[Alhadis]

Awesome! So is v2.1.4. Thanks for that! 🙏