Skip to content

Commit 016c706

Browse files
Merge pull request #204 from cloudinary/fix/html-image-destroy
fix: cleanup html layer on component unmount
2 parents 3c162cd + 798653e commit 016c706

3 files changed

Lines changed: 57 additions & 2 deletions

File tree

packages/html/__tests__/HtmlImageLayer.test.ts

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@ import {HtmlImageLayer} from "../src";
33
import {BaseAnalyticsOptions} from "../src/types";
44
import {responsive} from "../src/plugins/responsive";
55
import {placeholder} from "../src/plugins/placeholder";
6-
import {lazyload} from "../src/plugins/lazyload";
76
import {accessibility} from "../src/plugins/accessibility";
7+
import {PluginResponse} from "../types";
8+
import {cancelCurrentlyRunningPlugins} from "../src/utils/cancelCurrentlyRunningPlugins";
89

910
jest.useFakeTimers();
1011

@@ -51,4 +52,47 @@ describe('HtmlImageLayer tests', function () {
5152
await flushPromises();
5253
expect(img.src).toEqualAnalyticsToken('AXAABABD');
5354
});
55+
56+
it('should set image src twice if HtmlImageLayer instance won\'t be unmounted', async function () {
57+
const img = document.createElement('img');
58+
const pluginsState: any = {
59+
cleanupCallbacks: []
60+
};
61+
const spy = jest.spyOn(img, 'setAttribute');
62+
const dummyFailingPlugin = (): Promise<PluginResponse> => new Promise((resolve) => {
63+
pluginsState.cleanupCallbacks.push(() => {
64+
resolve('canceled')
65+
})
66+
});
67+
const dummyLazyLoadPlugin = (): Promise<PluginResponse> => new Promise((resolve) => {
68+
resolve({lazyload: true});
69+
});
70+
new HtmlImageLayer(img, cldImage, [dummyFailingPlugin]);
71+
new HtmlImageLayer(img, cldImage, [dummyLazyLoadPlugin]);
72+
cancelCurrentlyRunningPlugins(pluginsState);
73+
await flushPromises();
74+
expect(spy).toHaveBeenCalledTimes(2);
75+
});
76+
77+
it('should set image src only once if HtmlImageLayer instance will be unmounted', async function () {
78+
const img = document.createElement('img');
79+
const pluginsState: any = {
80+
cleanupCallbacks: []
81+
};
82+
const spy = jest.spyOn(img, 'setAttribute');
83+
const dummyFailingPlugin = (): Promise<PluginResponse> => new Promise((resolve) => {
84+
pluginsState.cleanupCallbacks.push(() => {
85+
resolve('canceled')
86+
})
87+
});
88+
const dummyLazyLoadPlugin = (): Promise<PluginResponse> => new Promise((resolve) => {
89+
resolve({lazyload: true});
90+
});
91+
const instance1 = new HtmlImageLayer(img, cldImage, [dummyFailingPlugin]);
92+
new HtmlImageLayer(img, cldImage, [dummyLazyLoadPlugin]);
93+
cancelCurrentlyRunningPlugins(pluginsState);
94+
instance1.unmount();
95+
await flushPromises();
96+
expect(spy).toHaveBeenCalledTimes(1);
97+
});
5498
});

packages/html/src/layers/htmlImageLayer.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {getAnalyticsOptions} from "../utils/analytics";
66

77
export class HtmlImageLayer{
88
private imgElement: any;
9+
private isMounted = true;
910
htmlPluginState: HtmlPluginState;
1011
constructor(element: HTMLImageElement | null, userCloudinaryImage: CloudinaryImage, plugins?: Plugins, baseAnalyticsOptions?: BaseAnalyticsOptions){
1112
this.imgElement = element;
@@ -14,6 +15,9 @@ export class HtmlImageLayer{
1415

1516
render(element, pluginCloudinaryImage, plugins, this.htmlPluginState, baseAnalyticsOptions)
1617
.then((pluginResponse)=>{ // when resolved updates the src
18+
if (!this.isMounted) {
19+
return;
20+
}
1721
this.htmlPluginState.pluginEventSubscription.forEach(fn=>{fn()});
1822
const analyticsOptions = getAnalyticsOptions(baseAnalyticsOptions, pluginResponse);
1923
this.imgElement.setAttribute('src', pluginCloudinaryImage.toURL(analyticsOptions));
@@ -30,8 +34,14 @@ export class HtmlImageLayer{
3034
const pluginCloudinaryImage = cloneDeep(userCloudinaryImage);
3135
render(this.imgElement, pluginCloudinaryImage, plugins, this.htmlPluginState)
3236
.then((pluginResponse)=>{
37+
if (!this.isMounted) {
38+
return;
39+
}
3340
const featuredAnalyticsOptions = getAnalyticsOptions(baseAnalyticsOptions, pluginResponse);
3441
this.imgElement.setAttribute('src', pluginCloudinaryImage.toURL(featuredAnalyticsOptions));
3542
});
3643
}
44+
unmount() {
45+
this.isMounted = false;
46+
}
3747
}

packages/react/src/AdvancedImage.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,12 @@ class AdvancedImage extends React.Component <ImgProps> {
9797
}
9898

9999
/**
100-
* On unmount, we cancel the currently running plugins.
100+
* On unmount, we cancel the currently running plugins, and destroy the html layer instance
101101
*/
102102
componentWillUnmount() {
103103
// Safely cancel running events on unmount.
104104
cancelCurrentlyRunningPlugins(this.htmlLayerInstance.htmlPluginState);
105+
this.htmlLayerInstance.unmount();
105106
}
106107

107108
render() {

0 commit comments

Comments
 (0)