-
-
Notifications
You must be signed in to change notification settings - Fork 353
Fix: V7 - JavaScript V9 - set undefined for updateProd when not set #4740
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 44 commits
04b6c06
0dc4bca
382fef6
33af36c
ad6a76c
15342a2
c267f9b
efea84f
65a10d5
028dbf9
a57459a
5de9867
ae65521
bd64169
39a81c7
3a4fd4c
d8088ad
6f71cda
55c1961
97853c6
dc2e123
9a0b21e
45ee6fa
816b506
25bf147
8584214
a26497e
499e5a3
578ad2c
d7fe6c6
b08c54f
19197f3
709697c
a5ddcda
219a73a
b3ba7a3
9f16e0b
ce42083
8e03349
a38cbcb
f56d96e
359153b
222077c
a5f021e
37e0c94
517161c
11dfba7
883a831
415f623
4dc429f
8d9f65b
7c8c3d1
0eb0901
0ad5d39
3fc3d9f
cb615e1
cf5c90f
8b93d90
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 |
---|---|---|
|
@@ -153,18 +153,18 @@ export function init(passedOptions: ReactNativeOptions): void { | |
*/ | ||
export function wrap<P extends Record<string, unknown>>( | ||
RootComponent: React.ComponentType<P>, | ||
options?: ReactNativeWrapperOptions | ||
options?: ReactNativeWrapperOptions | ||
lucas-zimerman marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
): React.ComponentType<P> { | ||
const profilerProps = { | ||
...(options?.profilerProps ?? {}), | ||
...(options?.profilerProps ?? { removeUpdateProps: true }), | ||
name: RootComponent.displayName ?? 'Root', | ||
updateProps: {} | ||
updateProps: options?.profilerProps | ||
|
||
}; | ||
|
||
const RootApp: React.FC<P> = (appProps) => { | ||
return ( | ||
<TouchEventBoundary {...(options?.touchEventBoundaryProps ?? {})}> | ||
<ReactNativeProfiler {...profilerProps}> | ||
<ReactNativeProfiler {...profilerProps }> | ||
lucas-zimerman marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
<FeedbackWidgetProvider> | ||
<RootComponent {...appProps} /> | ||
</FeedbackWidgetProvider> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
import { Profiler } from '@sentry/react'; | ||
import type { ProfilerProps } from '@sentry/react/build/types/profiler'; | ||
import type { ReactNativeWrapperOptions } from 'src/js/options'; | ||
|
||
import { ReactNativeProfiler } from '../../src/js/tracing'; | ||
|
||
jest.mock('@sentry/react', () => ({ | ||
Profiler: jest.fn(), | ||
})); | ||
|
||
describe('ReactNativeProfiler', () => { | ||
beforeEach(() => { | ||
jest.clearAllMocks(); | ||
}); | ||
|
||
it('removes updateProps if removeUpdateProps is true', () => { | ||
const props: ReactNativeWrapperOptions & { name: string; updateProps: undefined; removeUpdateProps: boolean } = { | ||
name: 'Root', | ||
updateProps: undefined, | ||
removeUpdateProps: true, | ||
}; | ||
|
||
new ReactNativeProfiler(props); | ||
|
||
expect(Profiler).toHaveBeenCalledWith( | ||
expect.not.objectContaining({ | ||
updateProps: expect.anything(), | ||
}), | ||
); | ||
|
||
expect(Profiler).not.toHaveBeenCalledWith( | ||
expect.objectContaining({ | ||
removeUpdateProps: expect.anything(), | ||
}), | ||
); | ||
}); | ||
|
||
it('does not remove updateProps if removeUpdateProps is undefined', () => { | ||
const props: ProfilerProps & { name: string } = { | ||
name: 'Root', | ||
updateProps: { prop: true }, | ||
}; | ||
|
||
new ReactNativeProfiler(props); | ||
|
||
expect(Profiler).toHaveBeenCalledWith( | ||
expect.objectContaining({ | ||
updateProps: { prop: true }, | ||
}), | ||
); | ||
|
||
expect(Profiler).not.toHaveBeenCalledWith( | ||
expect.objectContaining({ | ||
removeUpdateProps: expect.anything(), | ||
}), | ||
); | ||
}); | ||
|
||
it('does not remove updateProps if removeUpdateProps is false', () => { | ||
const props: ProfilerProps & { name: string; removeUpdateProps: boolean } = { | ||
name: 'Root', | ||
updateProps: { prop: true }, | ||
removeUpdateProps: false, | ||
}; | ||
|
||
new ReactNativeProfiler(props); | ||
|
||
expect(Profiler).toHaveBeenCalledWith( | ||
expect.objectContaining({ | ||
updateProps: { prop: true }, | ||
}), | ||
); | ||
expect(Profiler).not.toHaveBeenCalledWith( | ||
expect.objectContaining({ | ||
removeUpdateProps: expect.anything(), | ||
}), | ||
); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,109 @@ | ||
import { render } from '@testing-library/react-native'; | ||
import * as React from 'react'; | ||
|
||
import { wrap } from '../src/js/sdk'; | ||
|
||
jest.mock('../src/js/touchevents', () => ({ | ||
TouchEventBoundary: ({ children }: { children: React.ReactNode }) => ( | ||
// eslint-disable-next-line react/no-unknown-property | ||
<div testID={"touch-boundaryID"}>{children}</div> | ||
), | ||
})); | ||
|
||
jest.mock('../src/js/tracing', () => ({ | ||
ReactNativeProfiler: jest.fn(({ children }: { children: React.ReactNode }) => ( | ||
// eslint-disable-next-line react/no-unknown-property | ||
<div testID="profilerID">{children}</div> | ||
)), | ||
})); | ||
|
||
jest.mock('../src/js/feedback/FeedbackWidgetManager', () => ({ | ||
FeedbackWidgetProvider: ({ children }: { children: React.ReactNode }) => ( | ||
// eslint-disable-next-line react/no-unknown-property | ||
<div testID="feedback-widgetID">{children}</div> | ||
), | ||
})); | ||
|
||
import type { ReactNativeWrapperOptions } from 'src/js/options'; | ||
|
||
import { ReactNativeProfiler } from '../src/js/tracing'; | ||
describe('Sentry.wrap', () => { | ||
|
||
const DummyComponent: React.FC<{ value?: string }> = ({ value }) => <div>{value}</div>; | ||
|
||
it('should not enforce any keys on the wrapped component', () => { | ||
const Mock: React.FC<{ test: 23 }> = () => <></>; | ||
const ActualWrapped = wrap(Mock); | ||
|
||
expect(typeof ActualWrapped.defaultProps).toBe(typeof Mock.defaultProps); | ||
}); | ||
|
||
it('wraps components with Sentry wrappers', () => { | ||
const Wrapped = wrap(DummyComponent); | ||
const renderResult = render(<Wrapped value="wrapped" />); | ||
|
||
expect(renderResult.toJSON()).toMatchInlineSnapshot(` | ||
<div | ||
testID="touch-boundaryID" | ||
> | ||
<div | ||
testID="profilerID" | ||
> | ||
<div | ||
testID="feedback-widgetID" | ||
> | ||
<div> | ||
wrapped | ||
</div> | ||
</div> | ||
</div> | ||
</div> | ||
`); | ||
}); | ||
|
||
describe('ReactNativeProfiler', () => { | ||
it('uses given options when set', () => { | ||
const options: ReactNativeWrapperOptions = { | ||
profilerProps: { disabled: false, includeRender: true, includeUpdates: true }, | ||
}; | ||
const Wrapped = wrap(DummyComponent, options); | ||
render(<Wrapped value="wrapped" />); | ||
|
||
expect(ReactNativeProfiler).toHaveBeenCalledWith( | ||
expect.objectContaining({ | ||
name: 'Root', | ||
disabled: false, | ||
includeRender: true, | ||
includeUpdates: true | ||
}), | ||
expect.anything(), | ||
); | ||
|
||
expect(ReactNativeProfiler).not.toHaveBeenCalledWith( | ||
expect.objectContaining({ | ||
updateProps: expect.anything(), | ||
}) | ||
); | ||
|
||
expect(ReactNativeProfiler).not.toHaveBeenCalledWith( | ||
expect.objectContaining({ | ||
removeUpdateProps: expect.anything(), | ||
}) | ||
); | ||
}); | ||
|
||
it('updateProps and removeUpdateProps defined when updateProps is not provided', () => { | ||
const Wrapped = wrap(DummyComponent); | ||
render(<Wrapped value="wrapped" />); | ||
|
||
expect(ReactNativeProfiler).toHaveBeenCalledWith( | ||
expect.objectContaining({ | ||
name: 'Root', | ||
updateProps: undefined, | ||
removeUpdateProps: true, | ||
}), | ||
expect.anything(), | ||
); | ||
}); | ||
}); | ||
}); |
Uh oh!
There was an error while loading. Please reload this page.