Skip to content

Commit 4c46b6c

Browse files
authored
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 #10265 (a real issue that was missed). * Don't validate events if no plugins are injected
1 parent 171149a commit 4c46b6c

2 files changed

Lines changed: 31 additions & 7 deletions

File tree

src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -281,28 +281,47 @@ function expectMarkupMismatch(serverElement, clientElement) {
281281
return testMarkupMatch(serverElement, clientElement, false);
282282
}
283283

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+
284288
// When there is a test that renders on server and then on client and expects a logged
285289
// error, you want to see the error show up both on server and client. Unfortunately,
286290
// React refuses to issue the same error twice to avoid clogging up the console.
287291
// To get around this, we must reload React modules in between server and client render.
288-
let onAfterResetModules = null;
289292
function resetModules() {
293+
// First, reset the modules to load the client renderer.
290294
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+
291302
PropTypes = require('prop-types');
292303
React = require('react');
293304
ReactDOM = require('react-dom');
294-
ReactDOMServer = require('react-dom/server');
295305
ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
296-
ReactDOMNodeStream = require('react-dom/node-stream');
297306
ReactTestUtils = require('react-dom/test-utils');
298-
// TODO: can we express this test with only public API?
299-
ExecutionEnvironment = require('ExecutionEnvironment');
300307

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();
303320
if (typeof onAfterResetModules === 'function') {
304321
onAfterResetModules();
305322
}
323+
324+
ReactDOMNodeStream = require('react-dom/node-stream');
306325
}
307326

308327
describe('ReactDOMServerIntegration', () => {

src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,11 @@ if (__DEV__) {
6767
if (EventPluginRegistry.registrationNameModules.hasOwnProperty(name)) {
6868
return true;
6969
}
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+
}
7075
warnedProperties[name] = true;
7176
var lowerCasedName = name.toLowerCase();
7277

0 commit comments

Comments
 (0)