[react devtools] Don't check for NODE_ENV==='test' because it never is #25186
Conversation
| @@ -810,32 +810,28 @@ export function attach( | |||
| // Patching the console enables DevTools to do a few useful things: | |||
| // * Append component stacks to warnings and error messages | |||
| // * Disable logging during re-renders to inspect hooks (see inspectHooksOfFiber) | |||
| // | |||
| // Don't patch in test environments because we don't want to interfere with Jest's own console overrides. | |||
| if (process.env.NODE_ENV !== 'test') { | |||
There was a problem hiding this comment.
This condition never evaluates to true
There was a problem hiding this comment.
Ah good catch. We probably want to do if (!__TEST__) here instead
There was a problem hiding this comment.
This breaks these tests:
Test Suites: 7 failed, 27 passed, 34 total
Tests: 49 failed, 6 skipped, 383 passed, 438 total
Snapshots: 97 failed, 566 passed, 663 total
I haven't looked into why, but it looks like many tests assume that registerRendererWithConsole(renderer); will be called. Do you have any thoughts on whether we want registerRendererWithConsole to be called here?
There was a problem hiding this comment.
Yeah it should be OK to move registerRendererWithConsole out of the test because we're not actually overriding the console at all there, just adding some functions for the console override methods to use
There was a problem hiding this comment.
Looks like wrapping any section in if (!__TEST__) breaks at least that many tests. Thoughts on landing this as is?
There was a problem hiding this comment.
Sure do it :) Curious to know the tests that are breaking though, and why. Do you think you could figure that out?
There was a problem hiding this comment.
Yeah, I think when I explore making changes to not use window.__REACT_DEVTOOLS_GLOBAL_FOO__-type variables, I should be able to figure it out
* yarn test is not broken. * And, I added an else clause which was never hit in a test.
#25186) * remove useless condition
#25186) * remove useless condition
#25186) * remove useless condition
* 'main' of ssh://github.com/GrinZero/react: (51 commits) Flow: add simple explicit export types to Devtools (facebook#25251) [react devtools][easy] Centralize calls to patchConsoleUsingWindowValues (facebook#25222) Unwind the current workInProgress if it's suspended (facebook#25247) Add early exit to strict mode (facebook#25235) fix: prettier ignore removed and fixed (facebook#24811) Flow: enable unsafe-addition error (facebook#25242) Flow: upgrade to 0.132 (facebook#25244) Flow: fix Fiber typed as any (facebook#25241) Flow: ReactFiberHotReloading recursive type (facebook#25225) Add some test coverage for some error cases (facebook#25240) experimental_use(context) for server components and ssr (facebook#25226) Flow: upgrade to 0.131 (facebook#25224) Prevent infinite re-renders in StrictMode + Offscreen (facebook#25203) Flow: remove explicit object syntax (facebook#25223) Flow: upgrade to 0.127 (facebook#25221) Flow: enable exact_by_default (facebook#25220) [react devtools] Don't check for NODE_ENV==='test' because it never is (facebook#25186) [react devtools][easy] Change variable names, etc. (facebook#25211) Bump async from 2.6.3 to 2.6.4 in /fixtures/concurrent/time-slicing (facebook#24443) Flow: implicit-inexact-object=error (facebook#25210) ...
Summary
if (process.env.NODE_ENV !== 'test') {and attempt to only patch the console if we're not running a testprocess.env.NODE_ENV === 'development'for tests.Why not fix the condition? (e.g. check
JEST_WORKER_ID != null)How did you test this change?