Fix false "unknown property" warnings for events in SSR (#10272)
* Reset modules between importing client and server modules in the test Client and server renderers still share some modules, including injections. This means they have shared state in this test, potentially missing issues that would occur in real world. After this change, the client and server modules are completely isolated in the test. This makes the tests fail due to https://github.com/facebook/react/issues/10265 (a real issue that was missed). * Don't validate events if no plugins are injected
Dan Abramov committed
Jul 25, 2017 at 17:01 UTC
4c46b6c98acee5f55c7fc668c882678028e9de61
2 files changed
+31
-7
src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js
+26
-7
@@ -281,28 +281,47 @@ function expectMarkupMismatch(serverElement, clientElement) {
281
return testMarkupMatch(serverElement, clientElement, false);
282
}
283
284
+// TODO: this is a hack for testing dynamic injection. Remove this when we decide
285
+// how to do static injection instead.
286
+let onAfterResetModules = null;
287
+
288
// When there is a test that renders on server and then on client and expects a logged
289
// error, you want to see the error show up both on server and client. Unfortunately,
290
// React refuses to issue the same error twice to avoid clogging up the console.
291
// To get around this, we must reload React modules in between server and client render.
288
-let onAfterResetModules = null;
292
function resetModules() {
293
+ // First, reset the modules to load the client renderer.
294
jest.resetModuleRegistry();
295
+ if (typeof onAfterResetModules === 'function') {
296
+ onAfterResetModules();
297
+ }
298
+
299
+ // TODO: can we express this test with only public API?
300
+ ExecutionEnvironment = require('ExecutionEnvironment');
301
+
302
PropTypes = require('prop-types');
303
React = require('react');
304
ReactDOM = require('react-dom');
294
- ReactDOMServer = require('react-dom/server');
305
ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
296
- ReactDOMNodeStream = require('react-dom/node-stream');
306
ReactTestUtils = require('react-dom/test-utils');
298
- // TODO: can we express this test with only public API?
299
- ExecutionEnvironment = require('ExecutionEnvironment');
307
301
- // TODO: this is a hack for testing dynamic injection. Remove this when we decide
302
- // how to do static injection instead.
308
+ // Now we reset the modules again to load the server renderer.
309
+ // Resetting is important because we want to avoid any shared state
310
+ // influencing the tests.
311
+ jest.resetModuleRegistry();
312
+ if (typeof onAfterResetModules === 'function') {
313
+ onAfterResetModules();
314
+ }
315
+
316
+ ReactDOMServer = require('react-dom/server');
317
+
318
+ // Finally, reset modules one more time before importing the stream renderer.
319
+ jest.resetModuleRegistry();
320
if (typeof onAfterResetModules === 'function') {
321
onAfterResetModules();
322
}
323
+
324
+ ReactDOMNodeStream = require('react-dom/node-stream');
325
}
326
327
describe('ReactDOMServerIntegration', () => {
src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js
+5
@@ -67,6 +67,11 @@ if (__DEV__) {
67
if (EventPluginRegistry.registrationNameModules.hasOwnProperty(name)) {
68
return true;
69
}
70
+ if (EventPluginRegistry.plugins.length === 0 && name.indexOf('on') === 0) {
71
+ // If no event plugins have been injected, we might be in a server environment.
72
+ // Don't check events in this case.
73
+ return true;
74
+ }
75
warnedProperties[name] = true;
76
var lowerCasedName = name.toLowerCase();
77