From 9f5b0e3e95a00e98a2517ed2c0ef1eb7642b5813 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 28 Nov 2017 21:26:06 +0000 Subject: [PATCH 1/4] Deprecate injecting custom event plugins --- packages/events/EventPluginRegistry.js | 21 ++++++++++++++++++ .../react-dom/src/__tests__/ReactDOM-test.js | 22 +++++++++++++++++++ packages/react-dom/src/client/ReactDOM.js | 7 ++++++ 3 files changed, 50 insertions(+) diff --git a/packages/events/EventPluginRegistry.js b/packages/events/EventPluginRegistry.js index aef6a7354e5f..daa620de83c7 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,20 @@ 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 (${names}) 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.', + ); + } + } + var isOrderingDirty = false; for (var pluginName in injectedNamesToPlugins) { if (!injectedNamesToPlugins.hasOwnProperty(pluginName)) { diff --git a/packages/react-dom/src/__tests__/ReactDOM-test.js b/packages/react-dom/src/__tests__/ReactDOM-test.js index dde2608bfa3b..751ae758fcda 100644 --- a/packages/react-dom/src/__tests__/ReactDOM-test.js +++ b/packages/react-dom/src/__tests__/ReactDOM-test.js @@ -372,4 +372,26 @@ describe('ReactDOM', () => { delete global.__REACT_DEVTOOLS_GLOBAL_HOOK__; } }); + + // 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.', + ); + } + }); }); diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index 76de75a36e45..d257ac967986 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, }; From 6e2bca389439d9de12f47f141185256b3937efa3 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 28 Nov 2017 22:48:48 +0000 Subject: [PATCH 2/4] Fix up tests --- .../ReactBrowserEventEmitter-test.internal.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.internal.js b/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.internal.js index 6676fa7a029b..09d631e54830 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); From 73ce7c521d260415e158e6a3ca6c4fbc8cfef386 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 29 Nov 2017 14:07:45 +0000 Subject: [PATCH 3/4] Fix CI --- packages/events/EventPluginRegistry.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/events/EventPluginRegistry.js b/packages/events/EventPluginRegistry.js index daa620de83c7..639f28f2dc1a 100644 --- a/packages/events/EventPluginRegistry.js +++ b/packages/events/EventPluginRegistry.js @@ -233,11 +233,12 @@ export function injectEventPluginsByName( const names = Object.keys(injectedNamesToPlugins).join(', '); lowPriorityWarning( false, - `Injecting custom event plugins (${names}) is deprecated ` + + '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, ); } } From a7bd9031710255b063187b7665c62270b7d45fe8 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 29 Nov 2017 14:41:35 +0000 Subject: [PATCH 4/4] oh noes --- packages/events/EventPluginRegistry.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/events/EventPluginRegistry.js b/packages/events/EventPluginRegistry.js index 639f28f2dc1a..ebd527ebc1d9 100644 --- a/packages/events/EventPluginRegistry.js +++ b/packages/events/EventPluginRegistry.js @@ -238,7 +238,7 @@ export function injectEventPluginsByName( '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, + names, ); } }