Skip to content

Commit 68ded35

Browse files
danpeendanpeen
authored
fix(core): fix addSourceList can not support URL object type fetch request (#589)
* fix(core): fix addSourceList can not support URL object type fetch request * chore: add unit tests
1 parent f128576 commit 68ded35

File tree

6 files changed

+151
-46
lines changed

6 files changed

+151
-46
lines changed

‎packages/browser-vm/__tests__/fetch.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ describe('Init', () => {
77

88
// Fetch methods auto fix base url
99
const sourceList: Array<any> = [];
10-
const addSourceList = (n)=> sourceList.push(n);
10+
const addSourceList = (n) => sourceList.push(n);
1111
it('fetch methods', (next) => {
1212
const sandbox = new Sandbox({
1313
namespace: 'vue',
@@ -35,7 +35,7 @@ describe('Init', () => {
3535
// xhr methods auto fix base url
3636
it('xhr methods', () => {
3737
const sourceList: Array<any> = [];
38-
const addSourceList = (n)=> sourceList.push(n);
38+
const addSourceList = (n) => sourceList.push(n);
3939
const sandbox = new Sandbox({
4040
namespace: 'react',
4141
addSourceList: addSourceList,

‎packages/browser-vm/src/modules/network.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ export function networkModule(sandbox: Sandbox) {
7676
if (sandbox.options.addSourceList) {
7777
sandbox.options.addSourceList({
7878
tagName: 'fetch',
79-
url: input instanceof Request ? input.url : input,
79+
url: input,
8080
});
8181
}
8282
let controller;

‎packages/browser-vm/src/types.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,11 @@ export interface SandboxOptions {
2525
disableWith?: boolean;
2626
strictIsolation?: boolean;
2727
modules?: Array<Module>;
28-
addSourceList?: (sourceInfo: Array<{ tagName: string; url: string }> | { tagName: string; url: string }) => void;
28+
addSourceList?: (
29+
sourceInfo:
30+
| Array<{ tagName: string; url: string | URL | Request }>
31+
| { tagName: string; url: string | URL | Request },
32+
) => void;
2933
loaderOptions?: LoaderOptions;
3034
styleScopeId?: () => string;
3135
el?: () => Element | ShadowRoot | null;

‎packages/core/__tests__/sourceList.spec.ts

Lines changed: 111 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,32 +24,131 @@ describe('Core: preload plugin', () => {
2424
},
2525
],
2626
});
27-
let app = await GarfishInstance.loadApp('vue-app');
27+
const app = await GarfishInstance.loadApp('vue-app');
2828
await app?.mount();
29-
app?.addSourceList({tagName: 'fetch', url: "http://localhost/resources/scripts/fetch.js"});
30-
app?.addSourceList({tagName: 'fetch', url: "http://localhost/resources/scripts/no-entry.js"});
31-
app?.addSourceList({tagName: 'style', url: "http://localhost/resources/scripts/style.js"});
29+
app?.addSourceList({
30+
tagName: 'fetch',
31+
url: 'http://localhost/resources/scripts/fetch.js',
32+
});
33+
app?.addSourceList({
34+
tagName: 'fetch',
35+
url: 'http://localhost/resources/scripts/no-entry.js',
36+
});
37+
app?.addSourceList({
38+
tagName: 'fetch',
39+
url: new URL('http://localhost/resources/reactApp.html'),
40+
});
41+
app?.addSourceList({
42+
tagName: 'fetch',
43+
url: new Request('http://localhost/resources/vue3App.html'),
44+
});
45+
app?.addSourceList({
46+
tagName: 'style',
47+
url: 'http://localhost/resources/scripts/style.js',
48+
});
3249
app?.addSourceList([
33-
{tagName: 'fetch', url: "http://localhost/resources/scripts/fetch.js"},
34-
{tagName: 'script', url: "http://localhost/resources/index.js"},
50+
{ tagName: 'fetch', url: 'http://localhost/resources/scripts/fetch.js' },
51+
{ tagName: 'script', url: 'http://localhost/resources/index.js' },
3552
]);
53+
app?.addSourceList({
54+
tagName: 'fetch',
55+
url: '/resources/scripts/render.js',
56+
});
57+
app?.addSourceList({
58+
tagName: 'script',
59+
url: '/resources/scripts/render.js',
60+
});
3661

37-
expect(app!.sourceList.length).toBe(4);
62+
expect(app!.sourceList.length).toBe(7);
3863
expect(app!.sourceList[0]).toMatchObject({
3964
tagName: 'script',
40-
url: 'http://localhost/resources/scripts/no-entry.js'
65+
url: 'http://localhost/resources/scripts/no-entry.js',
4166
});
4267
expect(app!.sourceList[1]).toMatchObject({
4368
tagName: 'fetch',
44-
url: 'http://localhost/resources/scripts/fetch.js'
69+
url: 'http://localhost/resources/scripts/fetch.js',
4570
});
4671
expect(app!.sourceList[2]).toMatchObject({
47-
tagName: 'style',
48-
url: 'http://localhost/resources/scripts/style.js'
72+
tagName: 'fetch',
73+
url: new URL('http://localhost/resources/reactApp.html'),
4974
});
5075
expect(app!.sourceList[3]).toMatchObject({
76+
tagName: 'fetch',
77+
url: new Request('http://localhost/resources/vue3App.html'),
78+
});
79+
expect(app!.sourceList[4]).toMatchObject({
80+
tagName: 'style',
81+
url: 'http://localhost/resources/scripts/style.js',
82+
});
83+
expect(app!.sourceList[5]).toMatchObject({
5184
tagName: 'script',
52-
url: 'http://localhost/resources/index.js'
85+
url: 'http://localhost/resources/index.js',
86+
});
87+
expect(app!.sourceList[6]).toMatchObject({
88+
tagName: 'fetch',
89+
url: '/resources/scripts/render.js',
5390
});
5491
});
92+
93+
it('set disableSourceListCollect true will not collect sourceList', async () => {
94+
const GarfishInstance = new Garfish({});
95+
GarfishInstance.run({
96+
disablePreloadApp: false,
97+
apps: [
98+
{
99+
name: 'vue-app',
100+
entry: vueSubAppEntry,
101+
cache: true,
102+
disableSourceListCollect: true,
103+
},
104+
],
105+
});
106+
const app = await GarfishInstance.loadApp('vue-app');
107+
await app?.mount();
108+
109+
app?.addSourceList({
110+
tagName: 'fetch',
111+
url: 'http://localhost/resources/scripts/fetch.js',
112+
});
113+
expect(app!.sourceList.length).toBe(0);
114+
115+
await GarfishInstance.loadApp('vue-app', {
116+
disableSourceListCollect: false,
117+
});
118+
119+
app?.addSourceList({
120+
tagName: 'fetch',
121+
url: 'http://localhost/resources/scripts/fetch.js',
122+
});
123+
124+
expect(app!.appInfo.disableSourceListCollect).toBe(true);
125+
expect(app!.sourceList.length).toBe(0);
126+
127+
const app2 = await GarfishInstance.loadApp('vue-app', {
128+
cache: false,
129+
disableSourceListCollect: false,
130+
});
131+
132+
expect(app2).not.toBe(app);
133+
expect(app!.appInfo.disableSourceListCollect).toBe(true);
134+
expect(app2!.appInfo.disableSourceListCollect).toBe(false);
135+
expect(app2!.sourceList.length).toBe(1);
136+
137+
expect(app2!.sourceList[0]).toMatchObject({
138+
tagName: 'script',
139+
url: 'http://localhost/resources/scripts/no-entry.js',
140+
});
141+
142+
app2?.addSourceList({
143+
tagName: 'fetch',
144+
url: 'http://localhost/resources/scripts/fetch.js',
145+
});
146+
147+
expect(app2!.sourceList.length).toBe(2);
148+
expect(app2!.sourceList[1]).toMatchObject({
149+
tagName: 'fetch',
150+
url: 'http://localhost/resources/scripts/fetch.js',
151+
});
152+
});
153+
55154
});

‎packages/core/src/module/app.ts

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
sourceListTags,
2323
createAppContainer,
2424
setDocCurrentScript,
25+
getSourceURL,
2526
} from '@garfish/utils';
2627
import { Garfish } from '../garfish';
2728
import { interfaces } from '../interface';
@@ -73,9 +74,8 @@ export class App {
7374
public cjsModules: Record<string, any>;
7475
public htmlNode: HTMLElement | ShadowRoot;
7576
public customExports: Record<string, any> = {}; // If you don't want to use the CJS export, can use this
76-
public sourceList: Array<{ tagName: string; url: string }> = [];
77-
public sourceListMap: Map<string, { tagName: string; url: string }> =
78-
new Map();
77+
public sourceList: Array<{ tagName: string; url: string | URL | Request }> = [];
78+
public sourceListMap: Map<string, { tagName: string; url: string | URL | Request }> = new Map();
7979
public appInfo: AppInfo;
8080
public context: Garfish;
8181
public hooks: interfaces.AppHooks;
@@ -144,6 +144,7 @@ export class App {
144144
const url =
145145
entryManager.findAttributeValue(node, 'href') ||
146146
entryManager.findAttributeValue(node, 'src');
147+
147148
if (url) {
148149
this.addSourceList({
149150
tagName: node.tagName,
@@ -177,33 +178,25 @@ export class App {
177178

178179
addSourceList(
179180
sourceInfo:
180-
| Array<{ tagName: string; url: string }>
181-
| { tagName: string; url: string },
181+
| Array<{ tagName: string; url: string | URL | Request }>
182+
| { tagName: string; url: string | URL | Request },
182183
) {
183184
if (this.appInfo.disableSourceListCollect) return;
184185
if (Array.isArray(sourceInfo)) {
185186
const nSourceList = sourceInfo.filter((item) => {
186-
const dup = Object.assign({}, item);
187-
dup.url = dup.url.startsWith('/')
188-
? `${location.origin}${dup.url}`
189-
: dup.url;
190-
191-
if (!this.sourceListMap.has(dup.url) && dup.url.startsWith('http')) {
192-
this.sourceListMap.set(dup.url, dup);
187+
const url = getSourceURL(item.url);
188+
if (!this.sourceListMap.has(url) && url.startsWith('http')) {
189+
this.sourceListMap.set(url, item);
193190
return true;
194191
}
195192
return false;
196193
});
197194
this.sourceList = this.sourceList.concat(nSourceList);
198195
} else {
199-
const dup = Object.assign({}, sourceInfo);
200-
dup.url = dup.url.startsWith('/')
201-
? `${location.origin}${dup.url}`
202-
: dup.url;
203-
204-
if (!this.sourceListMap.get(dup.url) && dup.url.startsWith('http')) {
205-
this.sourceList.push(dup);
206-
this.sourceListMap.set(dup.url, dup);
196+
const url = getSourceURL(sourceInfo.url);
197+
if (!this.sourceListMap.get(url) && url.startsWith('http')) {
198+
this.sourceList.push(sourceInfo);
199+
this.sourceListMap.set(url, sourceInfo);
207200
}
208201
}
209202
}
@@ -625,7 +618,7 @@ export class App {
625618
isInline: jsManager.isInlineScript(),
626619
noEntry: toBoolean(
627620
entryManager.findAttributeValue(node, 'no-entry') ||
628-
this.isNoEntryScript(targetUrl),
621+
this.isNoEntryScript(targetUrl),
629622
),
630623
originScript: mockOriginScript,
631624
});

‎packages/utils/src/utils.ts

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
export const noop = () => {};
1+
export const noop = () => { };
22

33
export const objectToString = Object.prototype.toString;
44

@@ -113,11 +113,11 @@ export function error(error: string | Error) {
113113
export function validURL(str) {
114114
const pattern = new RegExp(
115115
'^(https?:\\/\\/)?' + // protocol
116-
'((([a-z\\d]([a-z\\d-]*[a-z\\d])*)\\.)+[a-z]{2,}|' + // domain name
117-
'((\\d{1,3}\\.){3}\\d{1,3}))' + // OR ip (v4) address
118-
'(\\:\\d+)?(\\/[-a-z\\d%_.~+]*)*' + // port and path
119-
'(\\?[;&a-z\\d%_.~+=-]*)?' + // query string
120-
'(\\#[-a-z\\d_]*)?$',
116+
'((([a-z\\d]([a-z\\d-]*[a-z\\d])*)\\.)+[a-z]{2,}|' + // domain name
117+
'((\\d{1,3}\\.){3}\\d{1,3}))' + // OR ip (v4) address
118+
'(\\:\\d+)?(\\/[-a-z\\d%_.~+]*)*' + // port and path
119+
'(\\?[;&a-z\\d%_.~+=-]*)?' + // query string
120+
'(\\#[-a-z\\d_]*)?$',
121121
'i',
122122
); // fragment locator
123123
return !!pattern.test(str);
@@ -402,7 +402,7 @@ export function setDocCurrentScript(
402402
) {
403403
if (!target) return noop;
404404
const el = document.createElement('script');
405-
405+
406406
if (!url && code) {
407407
el.textContent = code;
408408
}
@@ -414,7 +414,7 @@ export function setDocCurrentScript(
414414
if (async) {
415415
el.setAttribute('async', 'true');
416416
}
417-
417+
418418
const set = (val) => {
419419
try {
420420
if (define) {
@@ -434,7 +434,7 @@ export function setDocCurrentScript(
434434
};
435435

436436
set(el);
437-
return () => safeWrapper(()=> delete target.currentScript, true);
437+
return () => safeWrapper(() => delete target.currentScript, true);
438438
}
439439

440440
export function _extends(d, b) {
@@ -574,3 +574,12 @@ export async function createSourcemap(code: string, filename: string) {
574574
);
575575
return `//@ sourceMappingURL=${content}`;
576576
}
577+
578+
// await fetch(new URL('http://www.example.com/démonstration.html'));
579+
// await fetch(new Request('http://www.example.com/démonstration.html'));
580+
// await fetch('http://www.example.com/démonstration.html');
581+
export function getSourceURL(url: string | URL | Request): string {
582+
if (url instanceof URL) return url.href;
583+
if (url instanceof Request) return url.url;
584+
return url.startsWith('/') ? `${location.origin}${url}` : url;
585+
}

0 commit comments

Comments
 (0)