Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions ilc/server/app.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -257,13 +257,16 @@ describe('App', () => {
chai.expect(response.headers['x-custom-header']).to.be.undefined;
});

it('should emit JS preload Link headers for all SSR-rendered fragment entry bundles', async () => {
it('should only emit JS preload Link headers for apps that explicitly opt in via discoveryMetadata', async () => {
const { app: testApp, server: testServer } = await createTestServer({
apps: {
'@portal/primary': {
spaBundle: 'http://localhost/primary.js',
kind: 'primary',
ssr: { src: 'http://apps.test/primary' },
discoveryMetadata: {
preloadSpaBundle: true,
},
},
'@portal/regular': {
spaBundle: 'http://localhost/regular.js',
Expand All @@ -279,7 +282,7 @@ describe('App', () => {
chai.expect(response.headers.link).to.include(
'<http://localhost/primary.js>; rel="preload"; as="script"; nopush;',
);
chai.expect(response.headers.link).to.include(
chai.expect(response.headers.link).to.not.include(
'<http://localhost/regular.js>; rel="preload"; as="script"; nopush;',
);
chai.expect(response.headers.link).to.not.include('/_ilc/client.js');
Expand All @@ -288,20 +291,26 @@ describe('App', () => {
}
});

it('should include wrapper entry bundle in JS preload Link headers', async () => {
it('should include wrapper entry bundle in JS preload Link headers when both opt in', async () => {
const { app: testApp, server: testServer } = await createTestServer({
apps: {
'@portal/wrappedApp': {
spaBundle: 'http://localhost/wrapped.js',
kind: 'primary',
ssr: { src: 'http://apps.test/wrappedApp' },
wrappedWith: '@portal/wrapper',
discoveryMetadata: {
preloadSpaBundle: true,
},
},
'@portal/wrapper': {
spaBundle: 'http://localhost/wrapper.js',
kind: 'wrapper',
ssr: { src: 'http://apps.test/wrapper' },
props: { param1: 'value1' },
discoveryMetadata: {
preloadSpaBundle: true,
},
},
},
});
Expand Down
62 changes: 62 additions & 0 deletions ilc/server/tailor/configs-injector.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ describe('configs injector', () => {
firstApp: {
spaBundle: 'https://somewhere.com/firstAppSpaBundle.js',
cssBundle: 'https://somewhere.com/firstAppCssBundle.css',
discoveryMetadata: {
preloadSpaBundle: true,
preloadCssBundle: true,
},
dependencies: {
firstAppFirstDependency: 'https://somewhere.com/firstAppFirstDependency.js',
firstAppSecondDependency: 'https://somewhere.com/firstAppSecondDependency.js',
Expand All @@ -105,6 +109,10 @@ describe('configs injector', () => {
secondApp: {
spaBundle: 'https://somewhere.com/secondAppSpaBundle.js',
cssBundle: 'https://somewhere.com/secondAppCssBundle.css',
discoveryMetadata: {
preloadSpaBundle: true,
preloadCssBundle: true,
},
dependencies: {
secondAppFirstDependency: 'https://somewhere.com/secondAppFirstDependency.js',
secondAppSecondDependency: 'https://somewhere.com/secondAppSecondDependency.js',
Expand Down Expand Up @@ -436,6 +444,60 @@ describe('configs injector', () => {
);
});

it('should skip fragment assets that explicitly opt out of response-level preloads', () => {
context.run(
{
url: 'http://test/a?test=15',
domain: 'test.com',
requestId: 'requestId123',
path: '/a',
protocol: 'https',
},
() => {
const configsInjector = new ConfigsInjector(newrelic);
const nonPreloadedRegistryConfig = _.cloneDeep(registryConfig);
nonPreloadedRegistryConfig.apps.firstApp.discoveryMetadata = {
preloadSpaBundle: false,
preloadCssBundle: false,
};
nonPreloadedRegistryConfig.apps.secondApp.discoveryMetadata = {
preloadSpaBundle: false,
preloadCssBundle: true,
};

const request = {
registryConfig: nonPreloadedRegistryConfig,
router: {
getFragmentsContext: () => ({
firstApp__at__firstSlot: {
spaBundleUrl: nonPreloadedRegistryConfig.apps.firstApp.spaBundle,
wrapperConf: null,
},
secondApp__at__secondSlot: {
spaBundleUrl: nonPreloadedRegistryConfig.apps.secondApp.spaBundle,
wrapperConf: {
name: nonPreloadedRegistryConfig.apps.secondApp.wrappedWith,
},
},
}),
},
};
const template = {
styleRefs: ['https://somewhere.com/template-only.css'],
content: '<html><head></head><body></body></html>',
};

configsInjector.inject(request, template, { slots, reqUrl: '/test/route?a=15' });

chai.expect(request.scriptRefs).to.be.eql([]);
chai.expect(request.styleRefs).to.be.eql([
nonPreloadedRegistryConfig.apps.secondApp.cssBundle,
'https://somewhere.com/template-only.css',
]);
},
);
});

it('should allow setting attributes on html, head and body tags', () => {
context.run(
{
Expand Down
68 changes: 27 additions & 41 deletions ilc/server/tailor/configs-injector.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import urlJoin from 'url-join';
import { RoutingStrategy, type IntlAdapterConfig } from 'ilc-sdk/app';
import type { RouterMatch } from '../../common/types/Router';
import { encodeHtmlEntities, uniqueArray } from '../../common/utils';
import { appIdToNameAndSlot, encodeHtmlEntities, uniqueArray } from '../../common/utils';
import { HrefLangService } from '../services/HrefLangService';
import { CanonicalTagService } from '../services/CanonicalTagService';
import type { PatchedHttpRequest } from '../types/PatchedHttpRequest';
Expand All @@ -18,9 +18,9 @@ type BrowserTimingHeaderProvider = {
getBrowserTimingHeader(): string;
};

type FragmentPreloadContext = Record<string, PartialFragmentContextEntry>;
type FragmentPreloadContext = Record<string, FragmentContextEntry>;

type PartialFragmentContextEntry = {
type FragmentContextEntry = {
spaBundleUrl?: string;
wrapperConf?: {
name?: string;
Expand All @@ -35,8 +35,6 @@ type PreloadAssets = {
};

type RouteAssets = {
dependencies: Record<string, string>;
spaBundles: string[];
stylesheetLinks: string[];
};

Expand Down Expand Up @@ -140,14 +138,18 @@ export class ConfigsInjector {
): string[] {
const scriptRefs: string[] = [];

for (const fragmentContext of Object.values(fragmentsContext)) {
if (fragmentContext.spaBundleUrl) {
for (const [appId, fragmentContext] of Object.entries(fragmentsContext)) {
const appInfo = this.getAppByFragmentId(apps, appId);

if (fragmentContext.spaBundleUrl && this.shouldPreloadSpaBundle(appInfo)) {
scriptRefs.push(fragmentContext.spaBundleUrl);
}

const wrapperBundle = fragmentContext.wrapperConf?.name
? apps[fragmentContext.wrapperConf.name]?.spaBundle
: undefined;
const wrapperAppName = fragmentContext.wrapperConf?.name;
const wrapperBundle =
wrapperAppName && this.shouldPreloadSpaBundle(apps[wrapperAppName])
? apps[wrapperAppName]?.spaBundle
: undefined;

if (wrapperBundle) {
scriptRefs.push(wrapperBundle);
Expand All @@ -166,7 +168,7 @@ export class ConfigsInjector {

for (const slotData of Object.values(slots)) {
const appInfo = apps[slotData.appName];
const cssBundle = appInfo?.cssBundle;
const cssBundle = this.shouldPreloadCssBundle(appInfo) ? appInfo?.cssBundle : undefined;

if (cssBundle && !routeStyleRefs.includes(cssBundle)) {
routeStyleRefs.push(cssBundle);
Expand All @@ -177,15 +179,7 @@ export class ConfigsInjector {
}

private getRouteAssets(apps: Record<string, RegistryApp>, slots: InjectRoute['slots']) {
const appsDependencies: Record<string, string> = {};

for (const appInfo of Object.values(apps)) {
Object.assign(appsDependencies, appInfo.dependencies);
}

const routeAssets: RouteAssets = {
dependencies: {},
spaBundles: [],
stylesheetLinks: [],
};

Expand All @@ -196,17 +190,6 @@ export class ConfigsInjector {
continue;
}

for (const dependencyName of Object.keys(appInfo.dependencies ?? {})) {
const dependencyUrl = appsDependencies[dependencyName];
if (dependencyUrl) {
routeAssets.dependencies[dependencyName] = dependencyUrl;
}
}

if (appInfo.spaBundle && !routeAssets.spaBundles.includes(appInfo.spaBundle)) {
routeAssets.spaBundles.push(appInfo.spaBundle);
}

if (
appInfo.cssBundle &&
!routeAssets.stylesheetLinks.some((stylesheetLink) => stylesheetLink.includes(appInfo.cssBundle!))
Expand All @@ -217,14 +200,7 @@ export class ConfigsInjector {
}
}

const scriptRefs = uniqueArray([
this.getClientjsUrl(),
...routeAssets.spaBundles,
...Object.values(routeAssets.dependencies),
]).filter(Boolean);

return {
scriptLinks: scriptRefs.map((scriptRef) => this.wrapWithLinkToPreloadScript(scriptRef)),
stylesheetLinks: routeAssets.stylesheetLinks,
};
}
Expand Down Expand Up @@ -301,10 +277,6 @@ export class ConfigsInjector {
return `<script src="${url}" type="text/javascript" ${this.getCrossoriginAttribute(url)} async></script>`;
}

private wrapWithLinkToPreloadScript(url: string): string {
return `<link rel="preload" href="${url}" as="script" ${this.getCrossoriginAttribute(url)}>`;
}

private wrapWithFragmentStylesheetLink(url: string, fragmentId: string): string {
return `<link rel="stylesheet" href="${url}" data-fragment-id="${fragmentId}">`;
}
Expand Down Expand Up @@ -369,4 +341,18 @@ export class ConfigsInjector {
},
};
}

private shouldPreloadSpaBundle(appInfo?: RegistryApp): boolean {
return appInfo?.discoveryMetadata?.preloadSpaBundle === true;
}

private shouldPreloadCssBundle(appInfo?: RegistryApp): boolean {
return appInfo?.discoveryMetadata?.preloadCssBundle !== false;
}

private getAppByFragmentId(apps: Record<string, RegistryApp>, appId: string): RegistryApp | undefined {
const { appName } = appIdToNameAndSlot(appId);

return apps[appName] ?? apps[appName.replace(/^@portal\//, '')];
}
}
4 changes: 4 additions & 0 deletions ilc/server/types/RegistryConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import type { Route, SpecialRoute } from '../../common/types/Router';

export type App = {
dependencies?: Record<string, string | undefined>;
discoveryMetadata?: {
preloadSpaBundle?: boolean;
preloadCssBundle?: boolean;
};
kind?: string;
ssr?: {
timeout?: number;
Expand Down
4 changes: 4 additions & 0 deletions registry/server/config/transformConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ type Dict = Record<string, any>;
type TransformedApp = VersionedRecord<Omit<App, 'enforceDomain'>> & {
ssr: Dict | null;
dependencies: Dict | null;
discoveryMetadata: Dict | null;
props: Dict | null;
ssrProps: Dict | null;
enforceDomain?: string;
Expand All @@ -21,6 +22,7 @@ export type AppDto = Pick<
| 'kind'
| 'ssr'
| 'dependencies'
| 'discoveryMetadata'
| 'props'
| 'ssrProps'
| 'spaBundle'
Expand Down Expand Up @@ -52,6 +54,7 @@ export function transformApps(
versionId: appendDigest(app.versionId, 'app'),
ssr: parseJSON<TransformedApp['ssr']>(app.ssr),
dependencies: parseJSON<TransformedApp['dependencies']>(app.dependencies),
discoveryMetadata: parseJSON<TransformedApp['discoveryMetadata']>(app.discoveryMetadata),
props: parseJSON<TransformedApp['props']>(app.props),
ssrProps: parseJSON<TransformedApp['ssrProps']>(app.ssrProps),
enforceDomain: app.enforceDomain ? getDomainName(app.enforceDomain) : undefined,
Expand Down Expand Up @@ -84,6 +87,7 @@ export function transformApps(
'kind',
'ssr',
'dependencies',
'discoveryMetadata',
'props',
'ssrProps',
'spaBundle',
Expand Down
4 changes: 4 additions & 0 deletions registry/tests/config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ const example = {
name: '@portal/ncTestAppName',
spaBundle: 'http://localhost:1234/ncTestAppReactssr.js',
cssBundle: 'http://127.0.0.1:1234/ncTestAppReactssr.css',
discoveryMetadata: {
preloadSpaBundle: true,
preloadCssBundle: true,
},
configSelector: ['ncTestSharedPropsName'],
props: {
appConfigName: 'appCorrect',
Expand Down
Loading