From e3fd9f9e58869252e1a1c977fceb28b5a1fa2e3e Mon Sep 17 00:00:00 2001 From: Jack Westbrook Date: Tue, 10 Sep 2024 14:35:21 +0200 Subject: [PATCH] Fix: Resolution of css for plugins using loadPluginCSS (#93128) * fix(plugins): resolve loadPluginCss urls for filesystem and cdn hosted plugins * fix(plugins): should a registry lookup fail in getLoadPluginCssUrl fallback to relative path * refactor(plugins): rename var to id for legibility * test(plugins): add some extra test cases for getLoadPluginCssUrl function --- .../plugins/loader/systemjsHooks.test.ts | 28 ++++++++++++++++++- .../features/plugins/loader/systemjsHooks.ts | 28 ++++++++++++++++--- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/public/app/features/plugins/loader/systemjsHooks.test.ts b/public/app/features/plugins/loader/systemjsHooks.test.ts index 517c00855d3..eae508b0e71 100644 --- a/public/app/features/plugins/loader/systemjsHooks.test.ts +++ b/public/app/features/plugins/loader/systemjsHooks.test.ts @@ -6,7 +6,7 @@ jest.mock('./cache', () => ({ import { server } from './pluginLoader.mock'; import { SystemJS } from './systemjs'; -import { decorateSystemJSFetch, decorateSystemJSResolve } from './systemjsHooks'; +import { decorateSystemJSFetch, decorateSystemJSResolve, getLoadPluginCssUrl } from './systemjsHooks'; import { SystemJSWithLoaderHooks } from './types'; describe('SystemJS Loader Hooks', () => { @@ -44,6 +44,7 @@ describe('SystemJS Loader Hooks', () => { expect(source).toContain('var pluginPath = "/public/plugins/";'); }); }); + describe('decorateSystemJSResolve', () => { it('removes legacy wildcard from resolved url', () => { const id = '/public/plugins/my-datasource/styles.css!'; @@ -105,3 +106,28 @@ describe('SystemJS Loader Hooks', () => { }); }); }); + +describe('getLoadPluginCssUrl', () => { + test('should return a fallback path if SystemJS.entries is empty', () => { + const path = 'plugins/sample-plugin/styles/dark.css'; + const result = getLoadPluginCssUrl(path); + + expect(result).toBe(`/public/${path}`); + }); + + test('should return a resolved url if SystemJS a entry exists', () => { + SystemJS.set('http://localhost/public/plugins/sample-plugin/module.js', {}); + const path = 'plugins/sample-plugin/styles/dark.css'; + const result = getLoadPluginCssUrl(path); + + expect(result).toBe('http://localhost/public/plugins/sample-plugin/styles/dark.css'); + }); + + test('should return a resolved url for entries that live on a cdn', () => { + SystemJS.set('http://my-cdn.com/sample-cdn-plugin/1.0.0/public/plugins/sample-cdn-plugin/module.js', {}); + const path = 'plugins/sample-cdn-plugin/styles/dark.css'; + const result = getLoadPluginCssUrl(path); + + expect(result).toBe('http://my-cdn.com/sample-cdn-plugin/1.0.0/public/plugins/sample-cdn-plugin/styles/dark.css'); + }); +}); diff --git a/public/app/features/plugins/loader/systemjsHooks.ts b/public/app/features/plugins/loader/systemjsHooks.ts index de0308a2604..e460c170ae9 100644 --- a/public/app/features/plugins/loader/systemjsHooks.ts +++ b/public/app/features/plugins/loader/systemjsHooks.ts @@ -46,11 +46,10 @@ export function decorateSystemJSResolve( // CDN hosted plugins contain the version in the path so skip return isFileSystemModule ? resolveWithCache(cleanedUrl) : cleanedUrl; } catch (err) { - // Provide fallback for plugins that use `loadPluginCss` to load theme styles - // Regex only targets plugins on the filesystem. + // Provide fallback for plugins that use `loadPluginCss` to load theme styles. if (LOAD_PLUGIN_CSS_REGEX.test(id)) { - const prefixId = `${config.appSubUrl ?? ''}/public/${id}`; - const url = originalResolve.apply(this, [prefixId, parentUrl]); + const resolvedUrl = getLoadPluginCssUrl(id); + const url = originalResolve.apply(this, [resolvedUrl, parentUrl]); return resolveWithCache(url); } console.warn(`SystemJS: failed to resolve '${id}'`); @@ -85,3 +84,24 @@ function getBackWardsCompatibleUrl(url: string) { return hasValidFileExtension ? url : url + '.js'; } + +// This function takes the path used in loadPluginCss and attempts to resolve it +// by checking the SystemJS entries for a matching pluginId then using that entry to find the baseUrl. +// If no match is found then it returns a fallback attempt at a relative path. +export function getLoadPluginCssUrl(id: string) { + const pluginId = id.split('/')[1]; + let url = ''; + for (const [moduleId] of SystemJS.entries()) { + if (moduleId.includes(pluginId)) { + url = moduleId; + break; + } + } + + const index = url.lastIndexOf('/plugins'); + if (index === -1) { + return `${config.appSubUrl ?? ''}/public/${id}`; + } + const baseUrl = url.substring(0, index); + return `${baseUrl}/${id}`; +}