Skip to content

Commit 670fd2c

Browse files
davidchindavidchin
committed
refactor(core): CHECKOUT-4455 Stop resolving promise with load event
BREAKING CHANGE: Previously, when a script is loaded, we return the associated event object inside a promise object to the caller. With this change, we return an empty promise object instead. This change is necessary because we can't guarantee that scripts can be loaded in the same way across different browsers. For example, some browsers don't support `rel="preload"`, so as a fallback, we have to preload scripts using regular XHR calls. In that case, we don't have an event object to return to the caller. We could potentially mock the event object to keep the return values consistent. But considering it is not a very useful thing to return, we've decided to make a breaking change and return nothing instead.
1 parent 0fc52e7 commit 670fd2c

File tree

4 files changed

+37
-33
lines changed

4 files changed

+37
-33
lines changed

‎src/script-loader.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ describe('ScriptLoader', () => {
6464
const output = await loader.loadScript('https://code.jquery.com/jquery-3.2.1.min.js');
6565

6666
expect(output)
67-
.toBeInstanceOf(Event);
67+
.toBeUndefined();
6868
});
6969

7070
it('does not load same script twice', async () => {
@@ -229,7 +229,7 @@ describe('ScriptLoader', () => {
229229
const output = await loader.preloadScript('https://cdn.foobar.com/foo.min.js');
230230

231231
expect(output)
232-
.toBeInstanceOf(Event);
232+
.toBeUndefined();
233233
});
234234
});
235235
});

‎src/script-loader.ts

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { RequestSender, Response } from '@bigcommerce/request-sender';
1+
import { RequestSender } from '@bigcommerce/request-sender';
22

33
import BrowserSupport from './browser-support';
44

@@ -11,8 +11,8 @@ export interface PreloadScriptOptions {
1111
}
1212

1313
export default class ScriptLoader {
14-
private _scripts: { [key: string]: Promise<Event> } = {};
15-
private _preloadedScripts: { [key: string]: Promise<Event | Response> } = {};
14+
private _scripts: { [key: string]: Promise<void> } = {};
15+
private _preloadedScripts: { [key: string]: Promise<void> } = {};
1616

1717
/**
1818
* @internal
@@ -22,14 +22,14 @@ export default class ScriptLoader {
2222
private _requestSender: RequestSender
2323
) {}
2424

25-
loadScript(src: string, options?: LoadScriptOptions): Promise<Event> {
25+
loadScript(src: string, options?: LoadScriptOptions): Promise<void> {
2626
if (!this._scripts[src]) {
2727
this._scripts[src] = new Promise((resolve, reject) => {
2828
const script = document.createElement('script') as LegacyHTMLScriptElement;
2929
const { async = false } = options || {};
3030

31-
script.onload = event => resolve(event);
32-
script.onreadystatechange = event => resolve(event);
31+
script.onload = () => resolve();
32+
script.onreadystatechange = () => resolve();
3333
script.onerror = event => {
3434
delete this._scripts[src];
3535
reject(event);
@@ -44,11 +44,12 @@ export default class ScriptLoader {
4444
return this._scripts[src];
4545
}
4646

47-
loadScripts(urls: string[], options?: LoadScriptOptions): Promise<Event[]> {
48-
return Promise.all(urls.map(url => this.loadScript(url, options)));
47+
loadScripts(urls: string[], options?: LoadScriptOptions): Promise<void> {
48+
return Promise.all(urls.map(url => this.loadScript(url, options)))
49+
.then(() => undefined);
4950
}
5051

51-
preloadScript(url: string, options?: PreloadScriptOptions): Promise<Event | Response> {
52+
preloadScript(url: string, options?: PreloadScriptOptions): Promise<void> {
5253
if (!this._preloadedScripts[url]) {
5354
this._preloadedScripts[url] = new Promise((resolve, reject) => {
5455
const { prefetch = false } = options || {};
@@ -61,13 +62,13 @@ export default class ScriptLoader {
6162
preloadedScript.rel = rel;
6263
preloadedScript.href = url;
6364

64-
preloadedScript.onload = event => {
65-
resolve(event);
65+
preloadedScript.onload = () => {
66+
resolve();
6667
};
6768

68-
preloadedScript.onerror = event => {
69+
preloadedScript.onerror = () => {
6970
delete this._preloadedScripts[url];
70-
reject(event);
71+
reject();
7172
};
7273

7374
document.head.appendChild(preloadedScript);
@@ -76,7 +77,7 @@ export default class ScriptLoader {
7677
credentials: false,
7778
headers: { Accept: 'application/javascript' },
7879
})
79-
.then(resolve)
80+
.then(() => resolve())
8081
.catch(reject);
8182
}
8283
});
@@ -85,8 +86,9 @@ export default class ScriptLoader {
8586
return this._preloadedScripts[url];
8687
}
8788

88-
preloadScripts(urls: string[], options?: PreloadScriptOptions): Promise<Array<Event | Response>> {
89-
return Promise.all(urls.map(url => this.preloadScript(url, options)));
89+
preloadScripts(urls: string[], options?: PreloadScriptOptions): Promise<void> {
90+
return Promise.all(urls.map(url => this.preloadScript(url, options)))
91+
.then(() => undefined);
9092
}
9193
}
9294

‎src/stylesheet-loader.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ describe('StylesheetLoader', () => {
5555
const output = await loader.loadStylesheet('https://foo.bar/hello-world.css');
5656

5757
expect(output)
58-
.toBeInstanceOf(Event);
58+
.toBeUndefined();
5959
});
6060

6161
it('does not load same stylesheet twice', async () => {
@@ -166,7 +166,7 @@ describe('StylesheetLoader', () => {
166166
const output = await loader.preloadStylesheet('https://foo.bar/hello-world.css');
167167

168168
expect(output)
169-
.toBeInstanceOf(Event);
169+
.toBeUndefined();
170170
});
171171
});
172172
});

‎src/stylesheet-loader.ts

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { RequestSender, Response } from '@bigcommerce/request-sender';
1+
import { RequestSender } from '@bigcommerce/request-sender';
22

33
import BrowserSupport from './browser-support';
44

@@ -11,8 +11,8 @@ export interface PreloadStylesheetOptions {
1111
}
1212

1313
export default class StylesheetLoader {
14-
private _stylesheets: { [key: string]: Promise<Event> } = {};
15-
private _preloadedStylesheets: { [key: string]: Promise<Event | Response> } = {};
14+
private _stylesheets: { [key: string]: Promise<void> } = {};
15+
private _preloadedStylesheets: { [key: string]: Promise<void> } = {};
1616

1717
/**
1818
* @internal
@@ -22,13 +22,13 @@ export default class StylesheetLoader {
2222
private _requestSender: RequestSender
2323
) {}
2424

25-
loadStylesheet(src: string, options?: LoadStylesheetOptions): Promise<Event> {
25+
loadStylesheet(src: string, options?: LoadStylesheetOptions): Promise<void> {
2626
if (!this._stylesheets[src]) {
2727
this._stylesheets[src] = new Promise((resolve, reject) => {
2828
const stylesheet = document.createElement('link');
2929
const { prepend = false } = options || {};
3030

31-
stylesheet.onload = event => resolve(event);
31+
stylesheet.onload = () => resolve();
3232
stylesheet.onerror = event => {
3333
delete this._stylesheets[src];
3434
reject(event);
@@ -47,11 +47,12 @@ export default class StylesheetLoader {
4747
return this._stylesheets[src];
4848
}
4949

50-
loadStylesheets(urls: string[], options?: LoadStylesheetOptions): Promise<Event[]> {
51-
return Promise.all(urls.map(url => this.loadStylesheet(url, options)));
50+
loadStylesheets(urls: string[], options?: LoadStylesheetOptions): Promise<void> {
51+
return Promise.all(urls.map(url => this.loadStylesheet(url, options)))
52+
.then(() => undefined);
5253
}
5354

54-
preloadStylesheet(url: string, options?: PreloadStylesheetOptions): Promise<Event | Response> {
55+
preloadStylesheet(url: string, options?: PreloadStylesheetOptions): Promise<void> {
5556
if (!this._preloadedStylesheets[url]) {
5657
this._preloadedStylesheets[url] = new Promise((resolve, reject) => {
5758
const { prefetch = false } = options || {};
@@ -64,8 +65,8 @@ export default class StylesheetLoader {
6465
preloadedStylesheet.rel = prefetch ? 'prefetch' : 'preload';
6566
preloadedStylesheet.href = url;
6667

67-
preloadedStylesheet.onload = event => {
68-
resolve(event);
68+
preloadedStylesheet.onload = () => {
69+
resolve();
6970
};
7071

7172
preloadedStylesheet.onerror = event => {
@@ -79,7 +80,7 @@ export default class StylesheetLoader {
7980
credentials: false,
8081
headers: { Accept: 'text/css' },
8182
})
82-
.then(resolve)
83+
.then(() => resolve())
8384
.catch(reject);
8485
}
8586
});
@@ -88,7 +89,8 @@ export default class StylesheetLoader {
8889
return this._preloadedStylesheets[url];
8990
}
9091

91-
preloadStylesheets(urls: string[], options?: PreloadStylesheetOptions): Promise<Array<Event | Response>> {
92-
return Promise.all(urls.map(url => this.preloadStylesheet(url, options)));
92+
preloadStylesheets(urls: string[], options?: PreloadStylesheetOptions): Promise<void> {
93+
return Promise.all(urls.map(url => this.preloadStylesheet(url, options)))
94+
.then(() => undefined);
9395
}
9496
}

0 commit comments

Comments
 (0)