-
Notifications
You must be signed in to change notification settings - Fork 233
feat: removed filter-console dependency and fallback if process.env is not available #624
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
a16578a
e41c156
5657d97
eb77f48
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,24 +1,40 @@ | ||
import filterConsole from 'filter-console' | ||
const consoleFilters = [ | ||
/^The above error occurred in the <.*?> component:/, // error boundary output | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've slightly loosened what this will match, in case a |
||
/^Error: Uncaught .+/ // jsdom output | ||
] | ||
|
||
function suppressErrorOutput() { | ||
if (process.env.RHTL_DISABLE_ERROR_FILTERING) { | ||
return () => {} | ||
} | ||
Comment on lines
-4
to
-6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the breaking change I referred to in the PR description |
||
const originalError = console.error | ||
|
||
return filterConsole( | ||
[ | ||
/^The above error occurred in the <TestComponent> component:/, // error boundary output | ||
/^Error: Uncaught .+/ // jsdom output | ||
], | ||
{ | ||
methods: ['error'] | ||
const error = (...args: Parameters<typeof originalError>) => { | ||
const message = typeof args[0] === 'string' ? args[0] : null | ||
if (!message || !consoleFilters.some((filter) => filter.test(message))) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to call it out, |
||
originalError(...args) | ||
} | ||
) | ||
} | ||
|
||
console.error = error | ||
|
||
return () => { | ||
console.error = originalError | ||
} | ||
} | ||
|
||
function errorFilteringDisabled() { | ||
try { | ||
return !!process.env.RHTL_DISABLE_ERROR_FILTERING | ||
mpeyper marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} catch { | ||
return false | ||
} | ||
} | ||
|
||
function enableErrorOutputSuppression() { | ||
// Automatically registers console error suppression and restoration in supported testing frameworks | ||
if (typeof beforeEach === 'function' && typeof afterEach === 'function') { | ||
if ( | ||
typeof beforeEach === 'function' && | ||
typeof afterEach === 'function' && | ||
!errorFilteringDisabled() | ||
) { | ||
let restoreConsole!: () => void | ||
|
||
beforeEach(() => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
import { useEffect } from 'react' | ||
|
||
import { ReactHooksRenderer } from '../../types/react' | ||
|
||
// This verifies that if process.env is unavailable | ||
// then we still auto-wire up the afterEach for folks | ||
describe('skip auto cleanup (no process.env) tests', () => { | ||
const originalEnv = process.env | ||
let cleanupCalled = false | ||
let renderHook: ReactHooksRenderer['renderHook'] | ||
|
||
beforeAll(() => { | ||
process.env = { | ||
...process.env, | ||
get RHTL_SKIP_AUTO_CLEANUP(): string | undefined { | ||
throw new Error('expected') | ||
} | ||
} | ||
Comment on lines
+13
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I couldn't remove |
||
renderHook = (require('..') as ReactHooksRenderer).renderHook | ||
}) | ||
|
||
afterAll(() => { | ||
process.env = originalEnv | ||
}) | ||
|
||
test('first', () => { | ||
const hookWithCleanup = () => { | ||
useEffect(() => { | ||
return () => { | ||
cleanupCalled = true | ||
} | ||
}) | ||
} | ||
renderHook(() => hookWithCleanup()) | ||
}) | ||
|
||
test('second', () => { | ||
expect(cleanupCalled).toBe(true) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -142,51 +142,4 @@ describe('error hook tests', () => { | |
expect(result.error).toBe(undefined) | ||
}) | ||
}) | ||
|
||
describe('error output suppression', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this moved to a new file |
||
test('should allow console.error to be mocked', async () => { | ||
const consoleError = console.error | ||
console.error = jest.fn() | ||
|
||
try { | ||
const { rerender, unmount } = renderHook( | ||
(stage) => { | ||
useEffect(() => { | ||
console.error(`expected in effect`) | ||
return () => { | ||
console.error(`expected in unmount`) | ||
} | ||
}, []) | ||
console.error(`expected in ${stage}`) | ||
}, | ||
{ | ||
initialProps: 'render' | ||
} | ||
) | ||
|
||
act(() => { | ||
console.error('expected in act') | ||
}) | ||
|
||
await act(async () => { | ||
await new Promise((resolve) => setTimeout(resolve, 100)) | ||
console.error('expected in async act') | ||
}) | ||
|
||
rerender('rerender') | ||
|
||
unmount() | ||
|
||
expect(console.error).toBeCalledWith('expected in render') | ||
expect(console.error).toBeCalledWith('expected in effect') | ||
expect(console.error).toBeCalledWith('expected in act') | ||
expect(console.error).toBeCalledWith('expected in async act') | ||
expect(console.error).toBeCalledWith('expected in rerender') | ||
expect(console.error).toBeCalledWith('expected in unmount') | ||
expect(console.error).toBeCalledTimes(6) | ||
} finally { | ||
console.error = consoleError | ||
} | ||
}) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
// This verifies that if process.env is unavailable | ||
// then we still auto-wire up the afterEach for folks | ||
describe('error output suppression (no process.env) tests', () => { | ||
const originalEnv = process.env | ||
const originalConsoleError = console.error | ||
|
||
beforeAll(() => { | ||
process.env = { | ||
...process.env, | ||
get RHTL_DISABLE_ERROR_FILTERING(): string | undefined { | ||
throw new Error('expected') | ||
} | ||
} | ||
require('..') | ||
}) | ||
|
||
afterAll(() => { | ||
process.env = originalEnv | ||
}) | ||
|
||
test('should not patch console.error', () => { | ||
expect(console.error).not.toBe(originalConsoleError) | ||
}) | ||
}) | ||
|
||
export {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
import { useEffect } from 'react' | ||
|
||
import { ReactHooksRenderer } from '../../types/react' | ||
|
||
describe('error output suppression tests', () => { | ||
test('should not suppress relevant errors', () => { | ||
const consoleError = console.error | ||
console.error = jest.fn() | ||
|
||
const { suppressErrorOutput } = require('..') as ReactHooksRenderer | ||
|
||
try { | ||
const restoreConsole = suppressErrorOutput() | ||
|
||
console.error('expected') | ||
console.error(new Error('expected')) | ||
console.error('expected with args', new Error('expected')) | ||
|
||
restoreConsole() | ||
|
||
expect(console.error).toBeCalledWith('expected') | ||
expect(console.error).toBeCalledWith(new Error('expected')) | ||
expect(console.error).toBeCalledWith('expected with args', new Error('expected')) | ||
expect(console.error).toBeCalledTimes(3) | ||
} finally { | ||
console.error = consoleError | ||
} | ||
}) | ||
|
||
test('should allow console.error to be mocked', async () => { | ||
const { renderHook, act } = require('..') as ReactHooksRenderer | ||
const consoleError = console.error | ||
console.error = jest.fn() | ||
|
||
try { | ||
const { rerender, unmount } = renderHook( | ||
(stage) => { | ||
useEffect(() => { | ||
console.error(`expected in effect`) | ||
return () => { | ||
console.error(`expected in unmount`) | ||
} | ||
}, []) | ||
console.error(`expected in ${stage}`) | ||
}, | ||
{ | ||
initialProps: 'render' | ||
} | ||
) | ||
|
||
act(() => { | ||
console.error('expected in act') | ||
}) | ||
|
||
await act(async () => { | ||
await new Promise((resolve) => setTimeout(resolve, 100)) | ||
console.error('expected in async act') | ||
}) | ||
|
||
rerender('rerender') | ||
|
||
unmount() | ||
|
||
expect(console.error).toBeCalledWith('expected in render') | ||
expect(console.error).toBeCalledWith('expected in effect') | ||
expect(console.error).toBeCalledWith('expected in act') | ||
expect(console.error).toBeCalledWith('expected in async act') | ||
expect(console.error).toBeCalledWith('expected in rerender') | ||
expect(console.error).toBeCalledWith('expected in unmount') | ||
expect(console.error).toBeCalledTimes(6) | ||
} finally { | ||
console.error = consoleError | ||
} | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import ReactDOM from 'react-dom' | ||
import * as ReactDOM from 'react-dom' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This prevents the import relying on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so this works the same either way? interesting. i would expect you to need to add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's interesting because |
||
import { act } from 'react-dom/test-utils' | ||
|
||
import { RendererProps, RendererOptions } from '../types/react' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
import { useEffect } from 'react' | ||
|
||
import { ReactHooksRenderer } from '../../types/react' | ||
|
||
// This verifies that if process.env is unavailable | ||
// then we still auto-wire up the afterEach for folks | ||
describe('skip auto cleanup (no process.env) tests', () => { | ||
const originalEnv = process.env | ||
let cleanupCalled = false | ||
let renderHook: ReactHooksRenderer['renderHook'] | ||
|
||
beforeAll(() => { | ||
process.env = { | ||
...process.env, | ||
get RHTL_SKIP_AUTO_CLEANUP(): string | undefined { | ||
throw new Error('expected') | ||
} | ||
} | ||
renderHook = (require('..') as ReactHooksRenderer).renderHook | ||
}) | ||
|
||
afterAll(() => { | ||
process.env = originalEnv | ||
}) | ||
|
||
test('first', () => { | ||
const hookWithCleanup = () => { | ||
useEffect(() => { | ||
return () => { | ||
cleanupCalled = true | ||
} | ||
}) | ||
} | ||
renderHook(() => hookWithCleanup()) | ||
}) | ||
|
||
test('second', () => { | ||
expect(cleanupCalled).toBe(true) | ||
}) | ||
}) |
Uh oh!
There was an error while loading. Please reload this page.