Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bring back support for favicon.ico #20612

Merged
merged 8 commits into from
Jan 16, 2023
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
30 changes: 26 additions & 4 deletions code/lib/builder-manager/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -150,6 +160,7 @@ const starter: StarterFunction = async function* starterGeneratorFn({
const html = await renderHTML(
template,
title,
favicon,
customHead,
cssFiles,
jsFiles,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -223,6 +244,7 @@ const builder: BuilderFunction = async function* builderGeneratorFn({ startTime,
const html = await renderHTML(
template,
title,
favicon,
customHead,
cssFiles,
jsFiles,
Expand Down
5 changes: 4 additions & 1 deletion code/lib/builder-manager/src/utils/data.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -9,6 +9,8 @@ import { safeResolve } from './safeResolve';

export const getData = async (options: Options) => {
const refs = getRefs(options);
const favicon = options.presets.apply<string>('favicon').then((p) => basename(p));

const features = options.presets.apply<Record<string, string | boolean>>('features');
const logLevel = options.presets.apply<string>('logLevel');
const title = options.presets.apply<string>('title');
Expand All @@ -33,5 +35,6 @@ export const getData = async (options: Options) => {
instance,
config,
logLevel,
favicon,
};
};
2 changes: 2 additions & 0 deletions code/lib/builder-manager/src/utils/template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export async function getManagerMainTemplate() {
export const renderHTML = async (
template: Promise<string>,
title: Promise<string | false>,
favicon: Promise<string>,
customHead: Promise<string | false>,
cssFiles: string[],
jsFiles: string[],
Expand All @@ -66,6 +67,7 @@ export const renderHTML = async (
return render(templateRef, {
title: titleRef ? `${titleRef} - Storybook` : 'Storybook',
files: { js: jsFiles, css: cssFiles },
favicon: await favicon,
globals: {
FEATURES: JSON.stringify(await features, null, 2),
REFS: JSON.stringify(await refs, null, 2),
Expand Down
7 changes: 5 additions & 2 deletions code/lib/builder-manager/templates/template.ejs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@
<meta charset="utf-8" />

<title><%= typeof title !== 'undefined'? title : 'Storybook'%></title>

<link rel="icon" type="image/svg+xml" href="./favicon.svg">
<meta name="viewport" content="width=device-width, initial-scale=1" />

<% if (favicon.endsWith('.svg')) {%>
<link rel="icon" type="image/svg+xml" href="./<%= favicon %>">
<% } else if (favicon.endsWith('.ico')) { %>
<link rel="icon" type="image/x-icon" href="./<%= favicon %>">
<% } %>

<% if (typeof head !== 'undefined') { %> <%- head %> <% } %>

Expand Down
79 changes: 75 additions & 4 deletions code/lib/core-server/src/presets/common-preset.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import fs from 'fs-extra';
import { deprecate } from '@storybook/node-logger';
import { getPreviewBodyTemplate, getPreviewHeadTemplate, loadEnvs } from '@storybook/core-common';
import { pathExists, readFile } from 'fs-extra';
import { deprecate, logger } from '@storybook/node-logger';
import {
getDirectoryFromWorkingDir,
getPreviewBodyTemplate,
getPreviewHeadTemplate,
loadEnvs,
} from '@storybook/core-common';
import type {
CLIOptions,
IndexerOptions,
Expand All @@ -10,6 +15,72 @@ 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: Pick<Options, 'presets' | 'configDir' | 'staticDir'>
) => {
if (value) {
return value;
}
const staticDirs = await options.presets.apply<StorybookConfig['staticDirs']>('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);
}
}

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.join(', ')}
`);
}

return flatlist[0] || defaultFavicon;
}

return defaultFavicon;
};

export const babel = async (_: unknown, options: Options) => {
const { presets } = options;
Expand Down Expand Up @@ -104,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 [
Expand Down
127 changes: 127 additions & 0 deletions code/lib/core-server/src/presets/favicon.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
/// <reference types="@types/jest" />;

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<typeof m.favicon>[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'));
});
16 changes: 2 additions & 14 deletions code/lib/core-server/src/utils/server-statics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<StorybookConfig['staticDirs']>('staticDirs');
const faviconPath = await options.presets.apply<string>('favicon');

if (staticDirs && options.staticDir) {
throw new Error(dedent`
Expand Down Expand Up @@ -45,24 +43,14 @@ 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);
}
})
);
}

if (!hasCustomFavicon) {
router.use(favicon(defaultFavIcon));
}
router.use(favicon(faviconPath));
}

export const parseStaticDir = async (arg: string) => {
Expand Down