From 76b039618d052f3b1d4acdf9112e47067e582416 Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Fri, 13 Jan 2023 15:43:36 +0100 Subject: [PATCH 1/6] add support for favicon.ico again --- code/lib/builder-manager/src/index.ts | 30 ++++++-- code/lib/builder-manager/src/utils/data.ts | 5 +- .../lib/builder-manager/src/utils/template.ts | 2 + .../builder-manager/templates/template.ejs | 7 +- .../core-server/src/presets/common-preset.ts | 72 ++++++++++++++++++- .../core-server/src/utils/server-statics.ts | 16 +---- 6 files changed, 108 insertions(+), 24 deletions(-) diff --git a/code/lib/builder-manager/src/index.ts b/code/lib/builder-manager/src/index.ts index e5fc9ef7e590..cf34db65a5d5 100644 --- a/code/lib/builder-manager/src/index.ts +++ b/code/lib/builder-manager/src/index.ts @@ -119,8 +119,18 @@ const starter: StarterFunction = async function* starterGeneratorFn({ }) { logger.info('=> Starting manager..'); - const { config, customHead, features, instance, refs, template, title, logLevel, docsOptions } = - await getData(options); + const { + config, + favicon, + customHead, + features, + instance, + refs, + template, + title, + logLevel, + docsOptions, + } = await getData(options); yield; @@ -150,6 +160,7 @@ const starter: StarterFunction = async function* starterGeneratorFn({ const html = await renderHTML( template, title, + favicon, customHead, cssFiles, jsFiles, @@ -190,8 +201,18 @@ const builder: BuilderFunction = async function* builderGeneratorFn({ startTime, throw new Error('outputDir is required'); } logger.info('=> Building manager..'); - const { config, customHead, features, instance, refs, template, title, logLevel, docsOptions } = - await getData(options); + const { + config, + customHead, + favicon, + features, + instance, + refs, + template, + title, + logLevel, + docsOptions, + } = await getData(options); yield; const addonsDir = config.outdir; @@ -223,6 +244,7 @@ const builder: BuilderFunction = async function* builderGeneratorFn({ startTime, const html = await renderHTML( template, title, + favicon, customHead, cssFiles, jsFiles, diff --git a/code/lib/builder-manager/src/utils/data.ts b/code/lib/builder-manager/src/utils/data.ts index 4a1da5b5b867..b72b7120a3bd 100644 --- a/code/lib/builder-manager/src/utils/data.ts +++ b/code/lib/builder-manager/src/utils/data.ts @@ -1,4 +1,4 @@ -import { join } from 'path'; +import { basename, join } from 'path'; import type { DocsOptions, Options } from '@storybook/types'; import { getRefs } from '@storybook/core-common'; @@ -9,6 +9,8 @@ import { safeResolve } from './safeResolve'; export const getData = async (options: Options) => { const refs = getRefs(options); + const favicon = options.presets.apply('favicon').then((p) => basename(p)); + const features = options.presets.apply>('features'); const logLevel = options.presets.apply('logLevel'); const title = options.presets.apply('title'); @@ -33,5 +35,6 @@ export const getData = async (options: Options) => { instance, config, logLevel, + favicon, }; }; diff --git a/code/lib/builder-manager/src/utils/template.ts b/code/lib/builder-manager/src/utils/template.ts index 48d402a9328d..8cb8eff974a4 100644 --- a/code/lib/builder-manager/src/utils/template.ts +++ b/code/lib/builder-manager/src/utils/template.ts @@ -50,6 +50,7 @@ export async function getManagerMainTemplate() { export const renderHTML = async ( template: Promise, title: Promise, + favicon: Promise, customHead: Promise, cssFiles: string[], jsFiles: string[], @@ -66,6 +67,7 @@ export const renderHTML = async ( return render(templateRef, { title: titleRef ? `${titleRef} - Storybook` : 'Storybook', files: { js: jsFiles, css: cssFiles }, + favicon, globals: { FEATURES: JSON.stringify(await features, null, 2), REFS: JSON.stringify(await refs, null, 2), diff --git a/code/lib/builder-manager/templates/template.ejs b/code/lib/builder-manager/templates/template.ejs index 296f0a8fca8d..7ea940ed8b7c 100644 --- a/code/lib/builder-manager/templates/template.ejs +++ b/code/lib/builder-manager/templates/template.ejs @@ -4,10 +4,13 @@ <%= typeof title !== 'undefined'? title : 'Storybook'%> - - + <% if (favicon.endsWidth('.svg')) {%> + + <% } else if (favicon.endsWidth('.ico')) { %> + + <% } %> <% if (typeof head !== 'undefined') { %> <%- head %> <% } %> diff --git a/code/lib/core-server/src/presets/common-preset.ts b/code/lib/core-server/src/presets/common-preset.ts index aa9a2f06c9b9..651109c398e7 100644 --- a/code/lib/core-server/src/presets/common-preset.ts +++ b/code/lib/core-server/src/presets/common-preset.ts @@ -1,6 +1,11 @@ -import fs from 'fs-extra'; -import { deprecate } from '@storybook/node-logger'; -import { getPreviewBodyTemplate, getPreviewHeadTemplate, loadEnvs } from '@storybook/core-common'; +import fs, { pathExists } from 'fs-extra'; +import { deprecate, logger } from '@storybook/node-logger'; +import { + getDirectoryFromWorkingDir, + getPreviewBodyTemplate, + getPreviewHeadTemplate, + loadEnvs, +} from '@storybook/core-common'; import type { CLIOptions, IndexerOptions, @@ -10,6 +15,67 @@ import type { StorybookConfig, } from '@storybook/types'; import { loadCsf } from '@storybook/csf-tools'; +import { join } from 'path'; +import { dedent } from 'ts-dedent'; +import { parseStaticDir } from '../utils/server-statics'; + +const defaultFavIcon = require.resolve('@storybook/core-server/public/favicon.svg'); + +export const favicon = async (value: string, options: Options) => { + if (value) { + return value; + } + const staticDirs = await options.presets.apply('staticDirs'); + + const statics = staticDirs + ? staticDirs.map((dir) => (typeof dir === 'string' ? dir : `${dir.from}:${dir.to}`)) + : options.staticDir; + + if (statics && statics.length > 0) { + const lists = await Promise.all( + statics.map(async (dir) => { + const results = []; + const relativeDir = staticDirs + ? getDirectoryFromWorkingDir({ + configDir: options.configDir, + workingDir: process.cwd(), + directory: dir, + }) + : dir; + const { staticPath, targetEndpoint } = await parseStaticDir(relativeDir); + + if (targetEndpoint === '/') { + const url = 'favicon.svg'; + const path = join(staticPath, url); + if (await pathExists(path)) { + results.push(path); + } + } + if (targetEndpoint === '/') { + const url = 'favicon.ico'; + const path = join(staticPath, url); + if (await pathExists(path)) { + results.push(path); + } + } + }) + ); + + const flatlist = lists.reduce((l1, l2) => l1.concat(l2), []); + + if (flatlist.length > 1) { + logger.warn(dedent` + Looks like multiple favicons were detected. Using the first one. + + ${flatlist.map((f) => f.path).join(', ')} + `); + } + + return flatlist[0] || defaultFavIcon; + } + + return defaultFavIcon; +}; export const babel = async (_: unknown, options: Options) => { const { presets } = options; diff --git a/code/lib/core-server/src/utils/server-statics.ts b/code/lib/core-server/src/utils/server-statics.ts index 4c0c9931f7cc..2ecc3e8fc22d 100644 --- a/code/lib/core-server/src/utils/server-statics.ts +++ b/code/lib/core-server/src/utils/server-statics.ts @@ -9,11 +9,9 @@ import favicon from 'serve-favicon'; import { dedent } from 'ts-dedent'; -const defaultFavIcon = require.resolve('@storybook/core-server/public/favicon.svg'); - export async function useStatics(router: any, options: Options) { - let hasCustomFavicon = false; const staticDirs = await options.presets.apply('staticDirs'); + const faviconPath = await options.presets.apply('favicon'); if (staticDirs && options.staticDir) { throw new Error(dedent` @@ -45,14 +43,6 @@ export async function useStatics(router: any, options: Options) { chalk`=> Serving static files from {cyan ${staticDir}} at {cyan ${targetEndpoint}}` ); router.use(targetEndpoint, express.static(staticPath, { index: false })); - - if (!hasCustomFavicon && targetEndpoint === '/') { - const faviconPath = path.join(staticPath, 'favicon.svg'); - if (await pathExists(faviconPath)) { - hasCustomFavicon = true; - router.use(favicon(faviconPath)); - } - } } catch (e) { logger.warn(e.message); } @@ -60,9 +50,7 @@ export async function useStatics(router: any, options: Options) { ); } - if (!hasCustomFavicon) { - router.use(favicon(defaultFavIcon)); - } + router.use(favicon(faviconPath)); } export const parseStaticDir = async (arg: string) => { From db2e889b2488393bb74106f1e939681e5a847d21 Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Fri, 13 Jan 2023 16:10:31 +0100 Subject: [PATCH 2/6] fix --- code/lib/builder-manager/templates/template.ejs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/code/lib/builder-manager/templates/template.ejs b/code/lib/builder-manager/templates/template.ejs index 7ea940ed8b7c..88053e2dfb18 100644 --- a/code/lib/builder-manager/templates/template.ejs +++ b/code/lib/builder-manager/templates/template.ejs @@ -6,9 +6,9 @@ <%= typeof title !== 'undefined'? title : 'Storybook'%> - <% if (favicon.endsWidth('.svg')) {%> + <% if (favicon.endWith('.svg')) {%> - <% } else if (favicon.endsWidth('.ico')) { %> + <% } else if (favicon.endWith('.ico')) { %> <% } %> From aae03d542c89c5d518cb441c88599ec70a242b3e Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Fri, 13 Jan 2023 16:42:16 +0100 Subject: [PATCH 3/6] I keep using the keyboard wrong --- code/lib/builder-manager/templates/template.ejs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/code/lib/builder-manager/templates/template.ejs b/code/lib/builder-manager/templates/template.ejs index 88053e2dfb18..3507acd7b973 100644 --- a/code/lib/builder-manager/templates/template.ejs +++ b/code/lib/builder-manager/templates/template.ejs @@ -6,9 +6,9 @@ <%= typeof title !== 'undefined'? title : 'Storybook'%> - <% if (favicon.endWith('.svg')) {%> + <% if (favicon.endsWith('.svg')) {%> - <% } else if (favicon.endWith('.ico')) { %> + <% } else if (favicon.endsWith('.ico')) { %> <% } %> From d9b7ce899b2c5e7da6f6e970ccd2c062406734c5 Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Fri, 13 Jan 2023 17:02:23 +0100 Subject: [PATCH 4/6] it is friday --- code/lib/builder-manager/src/utils/template.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/lib/builder-manager/src/utils/template.ts b/code/lib/builder-manager/src/utils/template.ts index 8cb8eff974a4..a561f6de85da 100644 --- a/code/lib/builder-manager/src/utils/template.ts +++ b/code/lib/builder-manager/src/utils/template.ts @@ -67,7 +67,7 @@ export const renderHTML = async ( return render(templateRef, { title: titleRef ? `${titleRef} - Storybook` : 'Storybook', files: { js: jsFiles, css: cssFiles }, - favicon, + favicon: await favicon, globals: { FEATURES: JSON.stringify(await features, null, 2), REFS: JSON.stringify(await refs, null, 2), From 884b926d26b2aef3c391f61af73b09bc6ae82d61 Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Mon, 16 Jan 2023 13:09:40 +0100 Subject: [PATCH 5/6] rename --- code/lib/core-server/src/presets/common-preset.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/code/lib/core-server/src/presets/common-preset.ts b/code/lib/core-server/src/presets/common-preset.ts index 651109c398e7..27dbaf9772e2 100644 --- a/code/lib/core-server/src/presets/common-preset.ts +++ b/code/lib/core-server/src/presets/common-preset.ts @@ -19,7 +19,7 @@ import { join } from 'path'; import { dedent } from 'ts-dedent'; import { parseStaticDir } from '../utils/server-statics'; -const defaultFavIcon = require.resolve('@storybook/core-server/public/favicon.svg'); +const defaultFavicon = require.resolve('@storybook/core-server/public/favicon.svg'); export const favicon = async (value: string, options: Options) => { if (value) { @@ -71,10 +71,10 @@ export const favicon = async (value: string, options: Options) => { `); } - return flatlist[0] || defaultFavIcon; + return flatlist[0] || defaultFavicon; } - return defaultFavIcon; + return defaultFavicon; }; export const babel = async (_: unknown, options: Options) => { From f5c5ad9522fb8e75797e7e16fd4f96ff484050d5 Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Mon, 16 Jan 2023 14:53:46 +0100 Subject: [PATCH 6/6] fix code and add test --- .../core-server/src/presets/common-preset.ts | 15 ++- .../core-server/src/presets/favicon.test.ts | 127 ++++++++++++++++++ 2 files changed, 137 insertions(+), 5 deletions(-) create mode 100644 code/lib/core-server/src/presets/favicon.test.ts diff --git a/code/lib/core-server/src/presets/common-preset.ts b/code/lib/core-server/src/presets/common-preset.ts index 27dbaf9772e2..6787c3554845 100644 --- a/code/lib/core-server/src/presets/common-preset.ts +++ b/code/lib/core-server/src/presets/common-preset.ts @@ -1,4 +1,4 @@ -import fs, { pathExists } from 'fs-extra'; +import { pathExists, readFile } from 'fs-extra'; import { deprecate, logger } from '@storybook/node-logger'; import { getDirectoryFromWorkingDir, @@ -21,7 +21,10 @@ import { parseStaticDir } from '../utils/server-statics'; const defaultFavicon = require.resolve('@storybook/core-server/public/favicon.svg'); -export const favicon = async (value: string, options: Options) => { +export const favicon = async ( + value: string, + options: Pick +) => { if (value) { return value; } @@ -42,6 +45,7 @@ export const favicon = async (value: string, options: Options) => { directory: dir, }) : dir; + const { staticPath, targetEndpoint } = await parseStaticDir(relativeDir); if (targetEndpoint === '/') { @@ -58,16 +62,17 @@ export const favicon = async (value: string, options: Options) => { results.push(path); } } + + return results; }) ); - const flatlist = lists.reduce((l1, l2) => l1.concat(l2), []); if (flatlist.length > 1) { logger.warn(dedent` Looks like multiple favicons were detected. Using the first one. - ${flatlist.map((f) => f.path).join(', ')} + ${flatlist.join(', ')} `); } @@ -170,7 +175,7 @@ export const features = async ( export const storyIndexers = async (indexers?: StoryIndexer[]) => { const csfIndexer = async (fileName: string, opts: IndexerOptions) => { - const code = (await fs.readFile(fileName, 'utf-8')).toString(); + const code = (await readFile(fileName, 'utf-8')).toString(); return loadCsf(code, { ...opts, fileName }).parse(); }; return [ diff --git a/code/lib/core-server/src/presets/favicon.test.ts b/code/lib/core-server/src/presets/favicon.test.ts new file mode 100644 index 000000000000..2a3834e248ed --- /dev/null +++ b/code/lib/core-server/src/presets/favicon.test.ts @@ -0,0 +1,127 @@ +/// ; + +import { join } from 'path'; +import * as fs from 'fs-extra'; +import { logger } from '@storybook/node-logger'; +import * as m from './common-preset'; + +const defaultFavicon = require.resolve('@storybook/core-server/public/favicon.svg'); + +const createPath = (...p: string[]) => join(process.cwd(), ...p); +const createOptions = (locations: string[]): Parameters[1] => ({ + configDir: '', + presets: { + apply: async (extension: string, config: any) => { + switch (extension) { + case 'staticDirs': { + return locations.map((location) => ({ from: location, to: '/' })); + } + default: { + return config as any; + } + } + }, + }, +}); + +jest.mock('fs-extra', () => { + return { + pathExists: jest.fn((p: string) => { + return false; + }), + }; +}); + +jest.mock('@storybook/node-logger', () => { + return { + logger: { + warn: jest.fn(() => {}), + }, + }; +}); + +const pathExists = fs.pathExists as jest.Mock; + +test('with no staticDirs favicon should return default', async () => { + const options = createOptions([]); + + expect(await m.favicon(undefined, options)).toBe(defaultFavicon); +}); + +test('with staticDirs containing a single favicon.ico should return the found favicon', async () => { + const location = 'static'; + pathExists.mockImplementation((p: string) => { + if (p === createPath(location)) { + return true; + } + if (p === createPath(location, 'favicon.ico')) { + return true; + } + return false; + }); + const options = createOptions([location]); + + expect(await m.favicon(undefined, options)).toBe(createPath(location, 'favicon.ico')); +}); + +test('with staticDirs containing a single favicon.svg should return the found favicon', async () => { + const location = 'static'; + pathExists.mockImplementation((p: string) => { + if (p === createPath(location)) { + return true; + } + if (p === createPath(location, 'favicon.svg')) { + return true; + } + return false; + }); + const options = createOptions([location]); + + expect(await m.favicon(undefined, options)).toBe(createPath(location, 'favicon.svg')); +}); + +test('with staticDirs containing a multiple favicons should return the first favicon and warn', async () => { + const location = 'static'; + pathExists.mockImplementation((p: string) => { + if (p === createPath(location)) { + return true; + } + if (p === createPath(location, 'favicon.ico')) { + return true; + } + if (p === createPath(location, 'favicon.svg')) { + return true; + } + return false; + }); + const options = createOptions([location]); + + expect(await m.favicon(undefined, options)).toBe(createPath(location, 'favicon.svg')); + + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('multiple favicons')); +}); + +test('with multiple staticDirs containing a multiple favicons should return the first favicon and warn', async () => { + const locationA = 'static-a'; + const locationB = 'static-b'; + pathExists.mockImplementation((p: string) => { + if (p === createPath(locationA)) { + return true; + } + if (p === createPath(locationB)) { + return true; + } + if (p === createPath(locationA, 'favicon.ico')) { + return true; + } + if (p === createPath(locationB, 'favicon.svg')) { + return true; + } + return false; + }); + const options = createOptions([locationA, locationB]); + + expect(await m.favicon(undefined, options)).toBe(createPath(locationA, 'favicon.ico')); + + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('multiple favicons')); +});