Deprecate accessing jQuery.Event#originalEvent

[simonihmig]

Implements the deprecation message for user-land code accessing originalEvent on jQuery.Event instances, as proposed by RFC#294 (https://github.com/emberjs/rfcs/blob/master/text/0294-optional-jquery.md#introducing-ember-jquery-legacy-and-deprecating-jqueryevent-usage)

Relates to #16687 introducing the _JQUERY_INTEGRATION flag also used here.

[simonihmig]

Tests failing due to the yarn registry being down...

[simonihmig]

After making prettier and IE11 (no Proxy) happy, it's finally green now 😀

@rwjblue would love your review here!

[rwjblue]

Can you restructure this slightly, like:

import { DEBUG } from '@glimmer/env';

export default function addJQueryEventDeprecation(jqEvent) {
  if (!DEBUG || !HAS_NATIVE_PROXY) {
    return jqEvent;
  }

  return new Proxy(...);
}

This will ensure we always just use the jqEvent in production builds...

[simonihmig]

Ah, that totally makes sense! I was worried a bit about runtime overhead, but somehow didn't think of this... 🙄#dontCodeLateAtNight

I guess uglify (or rollup even before that?) will then strip it away at build time?

[rwjblue]

Yep, exactly!

[rwjblue]

Can you remove this function and inline the deprecate() call directly into addJQueryEventDeprecation? This will reduce the total byte size impact for production builds (this outer function will be stripped as well).

[rwjblue]

I generally like to add these at the top of the file, would you mind moving it up there?

[rwjblue]

Should we cache these?

Something like:

let boundFunctions = new Map();
// ...snip..
  if (!boundFunctions.has(name)) { 
    boundFucntions.set(name, target[name].bind(target));
  }
  
  return boundFunctions.get(name);

[simonihmig]

@rwjblue two tests in the build-tests scenario failed, which test for the __originalEvent property. I guess the tests run against a production build, is that right? Then that would make sense, as this property is only set when the Proxy-wrapper is applied, i.e. not in production anymore after the recent changes. Should I add another guard around these tests, to only run when DEBUG is set, or what would be the appropriate way to handle this?

[rwjblue]

Should I add another guard around these tests, to only run when DEBUG is set, or what would be the appropriate way to handle this?

Yes, I think that makes sense.

[simonihmig]

Fixed!

[rwjblue]

Awesome, thank you!