-
Notifications
You must be signed in to change notification settings - Fork 14
Image: support ImageSource with headers #8
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 all commits
32f1137
e2248d6
6429c1e
9ef7d28
7b9f633
d90e85c
26c1ae9
7c6f58e
33cb405
9edf19b
4227410
10df92e
9d6cbe7
42ef468
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 | ||
---|---|---|---|---|
|
@@ -56,7 +56,7 @@ exports[`components/Image prop "defaultSource" does not override "height" and "w | |||
exports[`components/Image prop "defaultSource" sets "height" and "width" styles if missing 1`] = ` | ||||
<div | ||||
class="css-view-175oi2r r-flexBasis-1mlwlqe r-overflow-1udh08x r-zIndex-417010" | ||||
style="height: 10px; width: 20px;" | ||||
style="width: 20px; height: 10px;" | ||||
> | ||||
<div | ||||
class="css-view-175oi2r r-backgroundColor-1niwhzg r-backgroundPosition-vvn4in r-backgroundRepeat-u6sd8q r-bottom-1p0dtai r-height-1pi2tsx r-left-1d2f490 r-position-u8s1d r-right-zchlnj r-top-ipm5af r-width-13qz1uu r-zIndex-1wyyakw r-backgroundSize-4gszlv" | ||||
|
@@ -329,14 +329,14 @@ exports[`components/Image prop "style" removes other unsupported View styles 1`] | |||
> | ||||
<div | ||||
class="css-view-175oi2r r-backgroundColor-1niwhzg r-backgroundPosition-vvn4in r-backgroundRepeat-u6sd8q r-bottom-1p0dtai r-height-1pi2tsx r-left-1d2f490 r-position-u8s1d r-right-zchlnj r-top-ipm5af r-width-13qz1uu r-zIndex-1wyyakw r-backgroundSize-4gszlv" | ||||
style="filter: url(#tint-57);" | ||||
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. Unfortunately these number change any time we add tests or render new Image components, because they are based on ID that is just a counter incremented with every Image component created
|
||||
style="filter: url(#tint-97);" | ||||
/> | ||||
<svg | ||||
style="position: absolute; height: 0px; visibility: hidden; width: 0px;" | ||||
> | ||||
<defs> | ||||
<filter | ||||
id="tint-57" | ||||
id="tint-97" | ||||
> | ||||
<feflood | ||||
flood-color="blue" | ||||
|
@@ -378,7 +378,7 @@ exports[`components/Image prop "style" supports "tintcolor" property (convert to | |||
> | ||||
<div | ||||
class="css-view-175oi2r r-backgroundColor-1niwhzg r-backgroundPosition-vvn4in r-backgroundRepeat-u6sd8q r-bottom-1p0dtai r-height-1pi2tsx r-left-1d2f490 r-position-u8s1d r-right-zchlnj r-top-ipm5af r-width-13qz1uu r-zIndex-1wyyakw r-backgroundSize-4gszlv" | ||||
style="background-image: url(https://google.com/favicon.ico); filter: url(#tint-56);" | ||||
style="background-image: url(https://google.com/favicon.ico); filter: url(#tint-95);" | ||||
/> | ||||
<img | ||||
alt="" | ||||
|
@@ -391,7 +391,7 @@ exports[`components/Image prop "style" supports "tintcolor" property (convert to | |||
> | ||||
<defs> | ||||
<filter | ||||
id="tint-56" | ||||
id="tint-95" | ||||
> | ||||
<feflood | ||||
flood-color="red" | ||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,11 @@ describe('components/Image', () => { | |
beforeEach(() => { | ||
ImageUriCache._entries = {}; | ||
window.Image = jest.fn(() => ({})); | ||
ImageLoader.load = jest | ||
.fn() | ||
.mockImplementation((source, onLoad, onError) => { | ||
onLoad({ source }); | ||
}); | ||
}); | ||
|
||
afterEach(() => { | ||
|
@@ -107,9 +112,6 @@ describe('components/Image', () => { | |
describe('prop "onLoad"', () => { | ||
test('is called after image is loaded from network', () => { | ||
jest.useFakeTimers(); | ||
ImageLoader.load = jest.fn().mockImplementation((_, onLoad, onError) => { | ||
onLoad(); | ||
}); | ||
const onLoadStartStub = jest.fn(); | ||
const onLoadStub = jest.fn(); | ||
const onLoadEndStub = jest.fn(); | ||
|
@@ -127,9 +129,6 @@ describe('components/Image', () => { | |
|
||
test('is called after image is loaded from cache', () => { | ||
jest.useFakeTimers(); | ||
ImageLoader.load = jest.fn().mockImplementation((_, onLoad, onError) => { | ||
onLoad(); | ||
}); | ||
const onLoadStartStub = jest.fn(); | ||
const onLoadStub = jest.fn(); | ||
const onLoadEndStub = jest.fn(); | ||
|
@@ -174,6 +173,35 @@ describe('components/Image', () => { | |
expect(onLoadEndStub.mock.calls.length).toBe(2); | ||
}); | ||
|
||
test('is called on update if "headers" are modified', () => { | ||
const onLoadStub = jest.fn(); | ||
const onLoadEndStub = jest.fn(); | ||
const { rerender } = render( | ||
<Image | ||
onLoad={onLoadStub} | ||
onLoadEnd={onLoadEndStub} | ||
source={{ | ||
uri: 'https://test.com/img.jpg', | ||
headers: { 'x-custom-header': 'abc123' } | ||
}} | ||
/> | ||
); | ||
act(() => { | ||
rerender( | ||
<Image | ||
onLoad={onLoadStub} | ||
onLoadEnd={onLoadEndStub} | ||
source={{ | ||
uri: 'https://test.com/img.jpg', | ||
headers: { 'x-custom-header': '123abc' } | ||
}} | ||
/> | ||
); | ||
}); | ||
expect(onLoadStub.mock.calls.length).toBe(2); | ||
expect(onLoadEndStub.mock.calls.length).toBe(2); | ||
marcaaron marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}); | ||
|
||
test('is not called on update if "uri" is the same', () => { | ||
const onLoadStartStub = jest.fn(); | ||
const onLoadStub = jest.fn(); | ||
|
@@ -225,6 +253,64 @@ describe('components/Image', () => { | |
expect(onLoadStub.mock.calls.length).toBe(1); | ||
expect(onLoadEndStub.mock.calls.length).toBe(1); | ||
}); | ||
|
||
// This test verifies that when source is declared in-line and the parent component | ||
// re-renders we aren't restarting the load process because the source is structurally equal | ||
test('is not called on update when "headers" and "uri" are not modified', () => { | ||
const onLoadStub = jest.fn(); | ||
const onLoadEndStub = jest.fn(); | ||
const { rerender } = render( | ||
<Image | ||
onLoad={onLoadStub} | ||
onLoadEnd={onLoadEndStub} | ||
source={{ | ||
uri: 'https://test.com/img.jpg', | ||
width: 1, | ||
height: 1, | ||
headers: { 'x-custom-header': 'abc123' } | ||
}} | ||
/> | ||
); | ||
act(() => { | ||
rerender( | ||
<Image | ||
onLoad={onLoadStub} | ||
onLoadEnd={onLoadEndStub} | ||
source={{ | ||
uri: 'https://test.com/img.jpg', | ||
width: 1, | ||
height: 1, | ||
headers: { 'x-custom-header': 'abc123' } | ||
}} | ||
/> | ||
); | ||
}); | ||
expect(onLoadStub.mock.calls.length).toBe(1); | ||
expect(onLoadEndStub.mock.calls.length).toBe(1); | ||
}); | ||
|
||
test('is not called for default source', () => { | ||
jest.useFakeTimers(); | ||
const onLoadStub = jest.fn(); | ||
render( | ||
<Image | ||
defaultSource="https://test.com/img-2.jpg" | ||
onLoad={onLoadStub} | ||
source="https://test.com/img.jpg" | ||
/> | ||
); | ||
jest.runOnlyPendingTimers(); | ||
expect(onLoadStub).toHaveBeenCalledTimes(1); | ||
expect(onLoadStub).toHaveBeenCalledWith( | ||
expect.objectContaining({ | ||
nativeEvent: { | ||
source: { | ||
uri: 'https://test.com/img.jpg' | ||
} | ||
} | ||
}) | ||
); | ||
}); | ||
}); | ||
|
||
describe('prop "resizeMode"', () => { | ||
|
@@ -245,7 +331,8 @@ describe('components/Image', () => { | |
'', | ||
{}, | ||
{ uri: '' }, | ||
{ uri: 'https://google.com' } | ||
{ uri: 'https://google.com' }, | ||
{ uri: 'https://google.com', headers: { 'x-custom-header': 'abc123' } } | ||
]; | ||
sources.forEach((source) => { | ||
expect(() => render(<Image source={source} />)).not.toThrow(); | ||
|
@@ -261,11 +348,6 @@ describe('components/Image', () => { | |
|
||
test('is set immediately if the image was preloaded', () => { | ||
const uri = 'https://yahoo.com/favicon.ico'; | ||
ImageLoader.load = jest | ||
.fn() | ||
.mockImplementationOnce((_, onLoad, onError) => { | ||
onLoad(); | ||
}); | ||
return Image.prefetch(uri).then(() => { | ||
const source = { uri }; | ||
const { container } = render(<Image source={source} />, { | ||
|
@@ -308,19 +390,33 @@ describe('components/Image', () => { | |
test('is correctly updated only when loaded if defaultSource provided', () => { | ||
const defaultUri = 'https://testing.com/preview.jpg'; | ||
const uri = 'https://testing.com/fullSize.jpg'; | ||
let loadCallback; | ||
ImageLoader.load = jest | ||
.fn() | ||
.mockImplementationOnce((_, onLoad, onError) => { | ||
loadCallback = onLoad; | ||
}); | ||
const calls = []; | ||
|
||
// Capture calls and resolve them after render | ||
ImageLoader.load = jest.fn().mockImplementation((source, onLoad) => { | ||
calls.push({ source, onLoad }); | ||
}); | ||
|
||
const { container } = render( | ||
<Image defaultSource={{ uri: defaultUri }} source={{ uri }} /> | ||
); | ||
|
||
// Both defaultSource and source are loaded at the same time | ||
// Since `defaultSource` is meant to be displayed until `source` loads | ||
// we resolve it first (otherwise it won't be displayed at all) | ||
act(() => { | ||
const call = calls.find(({ source }) => source.uri === defaultUri); | ||
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. Seems like the previous test had a snapshot before anything was resolved. Do we still need that? 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.
The loading behavior changed and so the test changed Previously the default source was not loaded using the ImageLoader |
||
call.onLoad({ source: call.source }); | ||
}); | ||
|
||
expect(container.firstChild).toMatchSnapshot(); | ||
|
||
// After a while the main source loads as well | ||
act(() => { | ||
loadCallback(); | ||
const call = calls.find(({ source }) => source.uri === uri); | ||
call.onLoad({ source: call.source }); | ||
}); | ||
|
||
expect(container.firstChild).toMatchSnapshot(); | ||
}); | ||
|
||
|
@@ -346,6 +442,67 @@ describe('components/Image', () => { | |
'http://localhost/static/[email protected]' | ||
); | ||
}); | ||
|
||
test('it correctly passes headers to ImageLoader', () => { | ||
const uri = 'https://google.com/favicon.ico'; | ||
const headers = { 'x-custom-header': 'abc123' }; | ||
const source = { uri, headers }; | ||
render(<Image source={source} />); | ||
|
||
expect(ImageLoader.load).toHaveBeenCalledWith( | ||
expect.objectContaining({ headers }), | ||
expect.any(Function), | ||
expect.any(Function) | ||
); | ||
}); | ||
|
||
test('it correctly passes uri to ImageLoader', () => { | ||
const uri = 'https://google.com/favicon.ico'; | ||
const source = { uri }; | ||
render(<Image source={source} />); | ||
|
||
expect(ImageLoader.load).toHaveBeenCalledWith( | ||
expect.objectContaining({ uri }), | ||
expect.any(Function), | ||
expect.any(Function) | ||
); | ||
}); | ||
|
||
// A common case is `source` declared as an inline object, which creates a | ||
// new object (with the same content) each time parent component renders | ||
test('it still loads the image if source object is changed', () => { | ||
ImageLoader.load.mockImplementation(() => {}); | ||
|
||
const releaseSpy = jest.spyOn(ImageLoader, 'abort'); | ||
|
||
const uri = 'https://google.com/favicon.ico'; | ||
const { rerender } = render(<Image source={{ uri }} />); | ||
rerender(<Image source={{ uri }} />); | ||
|
||
// when the underlying source didn't change we expect the initial request is not cancelled due to re-render | ||
expect(releaseSpy).not.toHaveBeenCalled(); | ||
}); | ||
|
||
test('falls back to default source when source or source.uri is removed', () => { | ||
const source = { uri: 'https://google.com/favicon.ico' }; | ||
const defaultSource = { uri: 'http://localhost/static/[email protected]' }; | ||
|
||
const { container, rerender } = render( | ||
<Image defaultSource={defaultSource} source={source} /> | ||
); | ||
|
||
rerender(<Image defaultSource={defaultSource} source={{ uri: '' }} />); | ||
expect(container.querySelector('img').src).toBe(defaultSource.uri); | ||
}); | ||
|
||
test('removes image if source or source.uri is removed and there is no default source', () => { | ||
const source = { uri: 'https://google.com/favicon.ico' }; | ||
|
||
const { container, rerender } = render(<Image source={source} />); | ||
|
||
rerender(<Image source={{ uri: '' }} />); | ||
expect(container.querySelector('img')).toBe(null); | ||
}); | ||
}); | ||
|
||
describe('prop "style"', () => { | ||
|
Uh oh!
There was an error while loading. Please reload this page.