Skip to content

Commit

Permalink
perf(assets-retry): reduce nullish coalescing operator (#4325)
Browse files Browse the repository at this point in the history
  • Loading branch information
@chenjiahan
chenjiahan authored Jan 5, 2025
1 parent 3fd33a4 commit aed200f
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 45 deletions.
50 changes: 34 additions & 16 deletions e2e/cases/assets/assets-retry/index.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ async function createRsbuildWithMiddleware(
options: PluginAssetsRetryOptions,
entry?: string,
port?: number,
assetPrefix?: string
assetPrefix?: string,
) {
const rsbuild = await dev({
cwd: __dirname,
Expand All @@ -94,9 +94,11 @@ async function createRsbuildWithMiddleware(
middlewares.unshift(...addMiddleWares);
},
],
...(assetPrefix ? {
assetPrefix
}: {})
...(assetPrefix
? {
assetPrefix,
}
: {}),
},
...(port
? {
Expand All @@ -111,10 +113,7 @@ async function createRsbuildWithMiddleware(
}
: {}),
output: {
sourceMap: {
css: false,
js: false,
},
sourceMap: false,
},
},
});
Expand Down Expand Up @@ -465,7 +464,11 @@ test('domain, onRetry and onFail options should work when retrying initial chunk
blockedMiddleware,
{
minify: true,
domain: [`http://localhost:${port}`, 'http://a.com/foo-path', 'http://b.com'],
domain: [
`http://localhost:${port}`,
'http://a.com/foo-path',
'http://b.com',
],
onRetry(context) {
console.info('onRetry', context);
},
Expand Down Expand Up @@ -541,7 +544,11 @@ test('domain, onRetry and onFail options should work when retrying async chunk f
blockedMiddleware,
{
minify: true,
domain: [`http://localhost:${port}`, 'http://a.com/foo-path', 'http://b.com'],
domain: [
`http://localhost:${port}`,
'http://a.com/foo-path',
'http://b.com',
],
onRetry(context) {
console.info('onRetry', context);
},
Expand Down Expand Up @@ -659,7 +666,9 @@ test('should work with addQuery boolean option', async ({ page }) => {
logger.level = 'log';
});

test('should work with addQuery boolean option when retrying async css chunk', async ({ page }) => {
test('should work with addQuery boolean option when retrying async css chunk', async ({
page,
}) => {
logger.level = 'verbose';
const { logs, restore } = proxyConsole();

Expand All @@ -678,7 +687,10 @@ test('should work with addQuery boolean option when retrying async css chunk', a
await gotoPage(page, rsbuild);
const asyncCompTestElement = page.locator('#async-comp-test');
await expect(asyncCompTestElement).toHaveText('Hello AsyncCompTest');
await expect(asyncCompTestElement).toHaveCSS('background-color', 'rgb(0, 0, 139)');
await expect(asyncCompTestElement).toHaveCSS(
'background-color',
'rgb(0, 0, 139)',
);

const blockedAsyncChunkResponseCount = count404ResponseByUrl(
logs,
Expand Down Expand Up @@ -901,14 +913,20 @@ test('onRetry and onFail options should work when multiple parallel retrying asy
await rsbuild.close();
});

test('should work when the first, second cdn are all failed and the third is success', async ({ page }) => {
test('should work when the first, second cdn are all failed and the third is success', async ({
page,
}) => {
// this is a real world case for assets-retry
const port = await getRandomPort();
const rsbuild = await createRsbuildWithMiddleware(
[],
{
minify: true,
domain: ['http://a.com/foo-path', 'http://b.com', `http://localhost:${port}`],
domain: [
'http://a.com/foo-path',
'http://b.com',
`http://localhost:${port}`,
],
addQuery: true,
onRetry(context) {
console.info('onRetry', context);
Expand All @@ -922,11 +940,11 @@ test('should work when the first, second cdn are all failed and the third is suc
},
undefined,
port,
'http://a.com/foo-path'
'http://a.com/foo-path',
);

await gotoPage(page, rsbuild);
const compTestElement = page.locator('#async-comp-test');
await expect(compTestElement).toHaveText('Hello AsyncCompTest');
await expect(compTestElement).toHaveCSS('background-color', 'rgb(0, 0, 139)');
})
});
35 changes: 15 additions & 20 deletions packages/plugin-assets-retry/src/runtime/asyncChunkRetry.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,22 +60,22 @@ const retryCollector: RetryCollector = {};
const retryCssCollector: RetryCollector = {};

function findCurrentDomain(url: string) {
const domainList = config.domain ?? [];
const domains = config.domain || [];
let domain = '';
for (let i = 0; i < domainList.length; i++) {
if (url.indexOf(domainList[i]) !== -1) {
domain = domainList[i];
for (let i = 0; i < domains.length; i++) {
if (url.indexOf(domains[i]) !== -1) {
domain = domains[i];
break;
}
}
return domain || window.origin;
}

function findNextDomain(url: string) {
const domainList = config.domain ?? [];
const domains = config.domain || [];
const currentDomain = findCurrentDomain(url);
const index = domainList.indexOf(currentDomain);
return domainList[(index + 1) % domainList.length] || url;
const index = domains.indexOf(currentDomain);
return domains[(index + 1) % domains.length] || url;
}

const postfixRE = /[?#].*$/;
Expand Down Expand Up @@ -146,7 +146,7 @@ function initRetry(chunkId: string, isCssAsyncChunk: boolean): Retry {
const originalQuery = getQueryFromUrl(originalSrcUrl);

const existRetryTimes = 0;
const nextDomain = config.domain?.[0] ?? window.origin;
const nextDomain = config.domain?.[0] || window.origin;

return {
nextDomain,
Expand Down Expand Up @@ -215,8 +215,8 @@ const originalEnsureChunk = __RUNTIME_GLOBALS_ENSURE_CHUNK__;
const originalGetChunkScriptFilename =
__RUNTIME_GLOBALS_GET_CHUNK_SCRIPT_FILENAME__;
const originalGetCssFilename =
__RUNTIME_GLOBALS_GET_MINI_CSS_EXTRACT_FILENAME__ ??
__RUNTIME_GLOBALS_GET_CSS_FILENAME__ ??
__RUNTIME_GLOBALS_GET_MINI_CSS_EXTRACT_FILENAME__ ||
__RUNTIME_GLOBALS_GET_CSS_FILENAME__ ||
(() => null);
const originalLoadScript = __RUNTIME_GLOBALS_LOAD_SCRIPT__;

Expand Down Expand Up @@ -335,11 +335,7 @@ function ensureChunk(chunkId: string): Promise<unknown> {
}
}

if (
config.domain &&
config.domain.length > 0 &&
config.domain.indexOf(nextDomain) === -1
) {
if (config.domain && config.domain.indexOf(nextDomain) === -1) {
throw error;
}

Expand Down Expand Up @@ -376,11 +372,10 @@ function loadScript() {

function loadStyleSheet(href: string, chunkId: ChunkId): string {
const retry = globalCurrRetryingCss[chunkId];
if (retry?.nextRetryUrl) {
return retry.nextRetryUrl;
}

return __RUNTIME_GLOBALS_PUBLIC_PATH__ + href;
return (
// biome-ignore lint/complexity/useOptionalChain: for less code
(retry && retry.nextRetryUrl) || __RUNTIME_GLOBALS_PUBLIC_PATH__ + href
);
}

function registerAsyncChunkRetry() {
Expand Down
18 changes: 9 additions & 9 deletions packages/plugin-assets-retry/src/runtime/initialChunkRetry.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,22 @@ declare global {
}

// this function is the same as async chunk retry
function findCurrentDomain(url: string, domainList: string[]) {
function findCurrentDomain(url: string, domains: string[]) {
let domain = '';
for (let i = 0; i < domainList.length; i++) {
if (url.indexOf(domainList[i]) !== -1) {
domain = domainList[i];
for (let i = 0; i < domains.length; i++) {
if (url.indexOf(domains[i]) !== -1) {
domain = domains[i];
break;
}
}
return domain || window.origin;
}

// this function is the same as async chunk retry
function findNextDomain(url: string, domainList: string[]) {
const currentDomain = findCurrentDomain(url, domainList);
const index = domainList.indexOf(currentDomain);
return domainList[(index + 1) % domainList.length] || url;
function findNextDomain(url: string, domains: string[]) {
const currentDomain = findCurrentDomain(url, domains);
const index = domains.indexOf(currentDomain);
return domains[(index + 1) % domains.length] || url;
}

function getRequestUrl(element: HTMLElement) {
Expand All @@ -56,7 +56,7 @@ function validateTargetInfo(
e: Event,
): { target: HTMLElement; tagName: string; url: string } | false {
const target: HTMLElement = e.target as HTMLElement;
const tagName = target.tagName?.toLocaleLowerCase();
const tagName = target.tagName.toLocaleLowerCase();
const allowTags = config.type!;
const url = getRequestUrl(target);
if (
Expand Down

1 comment on commit aed200f

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Ran ecosystem CI: Open

suite result
modernjs ❌ failure
plugins ✅ success
rspress ❌ failure
rslib ✅ success
examples ✅ success

Please sign in to comment.