Deprecate classList library

looks good to me

I think this should be added to the IE 9-10 CR https://www.drupal.org/node/2897971 and also this needs it's own CR for 8.6.x because it is possible that someone has added a library that depends on drupal.active-link and is depending on its classlist dependency for ie 9/10 support. In that instance they need to add the classlist dependency to their library.

I've created an issue on the IE9 compatibility module, which could alter the library to restore this dependency for sites that need it #3043990: classList JS library, and could provide its own classList library in the future when it's removed from core.

The patch in #9 looks perfect, I just re-rolled it against 8.9.x and removed the single quotes.
The only other change is the link in the deprecation notice should point to the CR for this issue, I created a quick draft change record optimistically hoping this can still make it into 8.8, and updated that url in the patch.

1. Shouldn't this require an addition to \Drupal\Tests\Listeners\DeprecationListenerTrait::getSkippedDeprecations?
2. Although there is an issue for the IE9 compatibility module, there are enough IE11 limitations to justify the polyfill getting its own contrib module in case there are custom modules/themes assuming these capabilities regardless of browser.
   

   1.Does not have support for classList on SVG or MathML elements.
   2. Does not support the second parameter for the toggle method
   3. Does not support multiple parameters for the add() & remove() methods
   4. Does not support assign to classList or the replace() method

   (better to have the contrib available that never gets used than not having a solution available)

3. The change record should document how this could potentially impact IE11
4. This should also get a release notes snippet.

#12.1 I don't think it does- unless the library is a dependency you don't get that warning so there's no need to suppress it.
#12.2 Looked again for these specific IE11 limitations, now that #3074267: Replace jQuery UI position() with PopperJS is landed there's a use of classList.toggle with a 2nd parameter in core/modules/quickedit/js/views/EntityToolbarView.es6.js
A contrib module would be a good idea but we also don't want to break an existing feature for IE11.

#10.3 Made a quick note in the CR but I'll update again once the contrib module is released.
#10.4 Added a release note to the IS

Looks like some of my feedback from #12 is irrelevant as I just noticed that this polyfill is loaded conditionally
browsers: { IE: 'lte IE 9', '!IE': false }, so this never polyfilled IE11 to begin with.

With that in mind, it's also worth noting that Quickedit uses a second param with classList.toggle(). This was a recent addition to core as part of adding the PopperJS library. This may mean this polyfill can't be deprecated, and actually needs to be loaded on more browsers.

A followup should be created to refactor the use of classList.toggle() with a second argument in Quickedit's EntityToolBarView to instead use a conditional that determines if add() or remove() is called, as opposed to using the param in toggle() to determine if the class is added or removed.

The IE11 specific stuff in the CR and RN I requested can be removed since we've since realized the polyfill is conditionally loaded.

The IE11 specific stuff in the CR and RN I requested can be removed since we've since realized the polyfill is conditionally loaded.

I agree, the polyfill is not currently loaded for IE11 so removing the polyfill won't make any difference.
The info about IE11 in the CR still seems useful, but I changed the wording to explain that the polyfill was only applied to IE9 and below.

A followup should be created to refactor the use of classList.toggle() with a second argument in Quickedit's EntityToolBarView to instead use a conditional that determines if add() or remove() is called, as opposed to using the param in toggle() to determine if the class is added or removed.

Better to address that existing browser bug separately than to block this issue. Created #3089752: Follow-up to #3074267 Refactor use of classList in quickedit for browser compatibility

Setting this back to NR

Everything looks good, I also provided a patch for #3089752: Follow-up to #3074267 Refactor use of classList in quickedit for browser compatibility

Discussed with @catch and we both feel this patch is good for 8.8.x BUT on close reading of #3086379: Deprecate html5shiv and remove usages in core that patch deprecated Html5Shiv which is only loaded in IE8 and less but did not remove core usages. Which is a different approach than this patch takes. Going to get @lauriii's opinion as frontend framework manager.

Discussed further with @catch - let's add the deprecation but also add it to getSkippedDeprecations() ala #3086379: Deprecate html5shiv and remove usages in core and then file a follow-up issue to actually do the removal in Drupal 9.0.0

As @catch said

if there are sites out there trying to support these older versions, we shouldn't actively break their attempts until 9.x

that patch deprecated Html5Shiv which is only loaded in IE8 and less but did not remove core usages. Which is a different approach than this patch takes.

That makes sense- just like html5shiv and matchmedia and domready there can be a Drupal 9 issue to actually remove the usage while this patch just gets the deprecation in.

Restored dependency to active-link
Added the warning suppression since it would cause a warning in LanguageSwitchingTest.

Updated IS and created #3090010: Remove classList in Drupal 9

Committed 592b6ae and pushed to 9.0.x. Thanks!
Committed and pushed d5fc5da747 to 8.9.x and 1bfa7c6dfc to 8.8.x. Thanks!

Now we can remove classlist completely from Drupal 9.