From 3e64b18540d474c88c440080681673faea5d3186 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 29 Nov 2017 17:48:16 +0000 Subject: [PATCH] Deprecate injecting custom event plugins (#11690) * Deprecate injecting custom event plugins * Fix up tests * Fix CI * oh noes --- packages/events/EventPluginRegistry.js | 22 +++++++++++++++++++ .../ReactBrowserEventEmitter-test.internal.js | 10 +++++++++ .../react-dom/src/__tests__/ReactDOM-test.js | 22 +++++++++++++++++++ packages/react-dom/src/client/ReactDOM.js | 7 ++++++ 4 files changed, 61 insertions(+) diff --git a/packages/events/EventPluginRegistry.js b/packages/events/EventPluginRegistry.js index aef6a7354e5f6..ebd527ebc1d97 100644 --- a/packages/events/EventPluginRegistry.js +++ b/packages/events/EventPluginRegistry.js @@ -15,10 +15,13 @@ import type { } from './PluginModuleType'; import invariant from 'fbjs/lib/invariant'; +import lowPriorityWarning from 'shared/lowPriorityWarning'; type NamesToPlugins = {[key: PluginName]: PluginModule}; type EventPluginOrder = null | Array; +var shouldWarnOnInjection = false; + /** * Injectable ordering of event plugins. */ @@ -29,6 +32,10 @@ var eventPluginOrder: EventPluginOrder = null; */ var namesToPlugins: NamesToPlugins = {}; +export function enableWarningOnInjection() { + shouldWarnOnInjection = true; +} + /** * Recomputes the plugin list using the injected plugins and plugin ordering. * @@ -221,6 +228,21 @@ export function injectEventPluginOrder( export function injectEventPluginsByName( injectedNamesToPlugins: NamesToPlugins, ): void { + if (__DEV__) { + if (shouldWarnOnInjection) { + const names = Object.keys(injectedNamesToPlugins).join(', '); + lowPriorityWarning( + false, + 'Injecting custom event plugins (%s) is deprecated ' + + 'and will not work in React 17+. Please update your code ' + + 'to not depend on React internals. The stack trace for this ' + + 'warning should reveal the library that is using them. ' + + 'See https://github.com/facebook/react/issues/11689 for a discussion.', + names, + ); + } + } + var isOrderingDirty = false; for (var pluginName in injectedNamesToPlugins) { if (!injectedNamesToPlugins.hasOwnProperty(pluginName)) { diff --git a/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.internal.js b/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.internal.js index 6676fa7a029b7..09d631e54830e 100644 --- a/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.internal.js @@ -132,11 +132,21 @@ describe('ReactBrowserEventEmitter', () => { idCallOrder = []; tapMoveThreshold = TapEventPlugin.tapMoveThreshold; + spyOnDev(console, 'warn'); EventPluginHub.injection.injectEventPluginsByName({ TapEventPlugin: TapEventPlugin, }); }); + afterEach(() => { + if (__DEV__) { + expect(console.warn.calls.count()).toBe(1); + expect(console.warn.calls.argsFor(0)[0]).toContain( + 'Injecting custom event plugins (TapEventPlugin) is deprecated', + ); + } + }); + it('should store a listener correctly', () => { registerSimpleTestHandler(); var listener = getListener(CHILD, ON_CLICK_KEY); diff --git a/packages/react-dom/src/__tests__/ReactDOM-test.js b/packages/react-dom/src/__tests__/ReactDOM-test.js index f10ebfb6dbcdb..b974c8bf41284 100644 --- a/packages/react-dom/src/__tests__/ReactDOM-test.js +++ b/packages/react-dom/src/__tests__/ReactDOM-test.js @@ -373,6 +373,28 @@ describe('ReactDOM', () => { } }); + // https://github.com/facebook/react/issues/11689 + it('should warn when attempting to inject an event plugin', () => { + spyOnDev(console, 'warn'); + ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.EventPluginHub.injection.injectEventPluginsByName( + { + TapEventPlugin: { + extractEvents() {}, + }, + }, + ); + if (__DEV__) { + expect(console.warn.calls.count()).toBe(1); + expect(console.warn.calls.argsFor(0)[0]).toContain( + 'Injecting custom event plugins (TapEventPlugin) is deprecated ' + + 'and will not work in React 17+. Please update your code ' + + 'to not depend on React internals. The stack trace for this ' + + 'warning should reveal the library that is using them. ' + + 'See https://github.com/facebook/react/issues/11689 for a discussion.', + ); + } + }); + it('throws in DEV if jsdom is destroyed by the time setState() is called', () => { spyOnDev(console, 'error'); class App extends React.Component { diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index 76de75a36e45d..d257ac9679863 100644 --- a/packages/react-dom/src/client/ReactDOM.js +++ b/packages/react-dom/src/client/ReactDOM.js @@ -1284,6 +1284,13 @@ const ReactDOM: Object = { }, }; +if (__DEV__) { + // Show deprecation warnings as we don't want to support injection forever. + // We do it now to let the internal injection happen without warnings. + // https://github.com/facebook/react/issues/11689 + EventPluginRegistry.enableWarningOnInjection(); +} + type RootOptions = { hydrate?: boolean, };