Skip to content

Commit

Permalink
Merge pull request #20458 from storybookjs/typescript-assets
Browse files Browse the repository at this point in the history
CLI: Do not use modern TS assets in legacy TS projects
  • Loading branch information
shilman authored Feb 13, 2023
2 parents b814979 + 2d9f6a7 commit 3288a19
Show file tree
Hide file tree
Showing 51 changed files with 77 additions and 51 deletions.
28 changes: 14 additions & 14 deletions code/frameworks/sveltekit/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,20 @@ Check out our [Frameworks API](https://storybook.js.org/blog/framework-api/) ann
All Svelte language features are supported out of the box, as Storybook uses the Svelte compiler underneath.
However SvelteKit has some [Kit-specific modules](https://kit.svelte.dev/docs/modules) that currently aren't supported. It's on our roadmap to support most of them soon:

| **Module** | **Status** | **Note** |
| ---------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| [`$app/environment`](https://kit.svelte.dev/docs/modules#$app-environment) | ✅ Supported | `version` is always empty in Storybook. |
| [`$app/forms`](https://kit.svelte.dev/docs/modules#$app-forms) | ⏳ Planned for 7.1 | Will use mocks. Tracked in [#20999](https://github.com/storybookjs/storybook/issues/20999) |
| [`$app/navigation`](https://kit.svelte.dev/docs/modules#$app-navigation) | ⏳ Planned for 7.1 | Will use mocks. Tracked in [#20999](https://github.com/storybookjs/storybook/issues/20999) |
| [`$app/paths`](https://kit.svelte.dev/docs/modules#$app-paths) | ✅ Supported | Requires SvelteKit 1.4.0 or newer |
| [`$app/stores`](https://kit.svelte.dev/docs/modules#$app-stores) | ✅ Supported | Mocks planned for 7.1, so you can set different store values per story. |
| [`$env/dynamic/private`](https://kit.svelte.dev/docs/modules#$env-dynamic-private) | ⛔ Not supported | They are meant to only be available server-side, and Storybook renders all components on the client. |
| [`$env/dynamic/public`](https://kit.svelte.dev/docs/modules#$env-dynamic-public) | 🚧 Partially supported | Only supported in development mode. Storybook is built as a static app with no server-side API so cannot dynamically serve content. |
| [`$env/static/private`](https://kit.svelte.dev/docs/modules#$env-static-private) | ⛔ Not supported | They are meant to only be available server-side, and Storybook renders all components on the client. |
| [`$env/static/public`](https://kit.svelte.dev/docs/modules#$env-static-public) | ✅ Supported | |
| [`$lib`](https://kit.svelte.dev/docs/modules#$lib) | ✅ Supported | |
| [`$service-worker`](https://kit.svelte.dev/docs/modules#$service-worker) | ⛔ Not supported | They are only meant to be used in service workers |
| [`@sveltejs/kit/*`](https://kit.svelte.dev/docs/modules#sveltejs-kit) | ✅ Supported | |
| **Module** | **Status** | **Note** |
| ---------------------------------------------------------------------------------- | ---------------------- | ----------------------------------------------------------------------------------------------------------------------------------- |
| [`$app/environment`](https://kit.svelte.dev/docs/modules#$app-environment) | ✅ Supported | `version` is always empty in Storybook. |
| [`$app/forms`](https://kit.svelte.dev/docs/modules#$app-forms) | ⏳ Planned for 7.1 | Will use mocks. Tracked in [#20999](https://github.com/storybookjs/storybook/issues/20999) |
| [`$app/navigation`](https://kit.svelte.dev/docs/modules#$app-navigation) | ⏳ Planned for 7.1 | Will use mocks. Tracked in [#20999](https://github.com/storybookjs/storybook/issues/20999) |
| [`$app/paths`](https://kit.svelte.dev/docs/modules#$app-paths) | ✅ Supported | Requires SvelteKit 1.4.0 or newer |
| [`$app/stores`](https://kit.svelte.dev/docs/modules#$app-stores) | ✅ Supported | Mocks planned for 7.1, so you can set different store values per story. |
| [`$env/dynamic/private`](https://kit.svelte.dev/docs/modules#$env-dynamic-private) | ⛔ Not supported | They are meant to only be available server-side, and Storybook renders all components on the client. |
| [`$env/dynamic/public`](https://kit.svelte.dev/docs/modules#$env-dynamic-public) | 🚧 Partially supported | Only supported in development mode. Storybook is built as a static app with no server-side API so cannot dynamically serve content. |
| [`$env/static/private`](https://kit.svelte.dev/docs/modules#$env-static-private) | ⛔ Not supported | They are meant to only be available server-side, and Storybook renders all components on the client. |
| [`$env/static/public`](https://kit.svelte.dev/docs/modules#$env-static-public) | ✅ Supported | |
| [`$lib`](https://kit.svelte.dev/docs/modules#$lib) | ✅ Supported | |
| [`$service-worker`](https://kit.svelte.dev/docs/modules#$service-worker) | ⛔ Not supported | They are only meant to be used in service workers |
| [`@sveltejs/kit/*`](https://kit.svelte.dev/docs/modules#sveltejs-kit) | ✅ Supported | |

This is just the beginning. We're close to adding basic support for many of the SvelteKit features. Longer term we're planning on making it an even better experience to [build](https://storybook.js.org/docs/7.0/react/writing-stories/introduction), [test](https://storybook.js.org/docs/7.0/react/writing-tests/introduction) and [document](https://storybook.js.org/docs/7.0/react/writing-docs/introduction) all the SvelteKit goodies like [pages](https://kit.svelte.dev/docs/routing), [forms](https://kit.svelte.dev/docs/form-actions) and [layouts](https://kit.svelte.dev/docs/routing#layout) in Storybook, while still integrating with all the addons and workflows you know and love.

Expand Down
29 changes: 22 additions & 7 deletions code/lib/cli/src/detect.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as fs from 'fs';

import { logger } from '@storybook/node-logger';
import { getBowerJson } from './helpers';
import { detect, detectFrameworkPreset, detectLanguage, isStorybookInstalled } from './detect';
import { ProjectType, SUPPORTED_RENDERERS, SupportedLanguage } from './project_types';
Expand All @@ -21,6 +21,8 @@ jest.mock('path', () => ({
join: jest.fn((_, p) => p),
}));

jest.mock('@storybook/node-logger');

const MOCK_FRAMEWORK_FILES: {
name: string;
files: Record<'package.json', PackageJsonWithMaybeDeps> | Record<string, string>;
Expand Down Expand Up @@ -298,27 +300,40 @@ describe('Detect', () => {
expect(detect(undefined)).toBe(ProjectType.UNDETECTED);
});

it(`should return language legacy typescript if the dependency is present`, () => {
it(`should return language javascript if the TS dependency is present but less than minimum supported`, () => {
(logger.warn as jest.MockedFunction<typeof logger.warn>).mockClear();
expect(detectLanguage({ dependencies: { typescript: '1.0.0' } })).toBe(
SupportedLanguage.TYPESCRIPT_LEGACY
SupportedLanguage.JAVASCRIPT
);
expect(logger.warn).toHaveBeenCalledWith(
'Detected TypeScript < 3.8, populating with JavaScript examples'
);
});

it(`should return language typescript-3-8 if the TS dependency is >=3.8 and <4.9`, () => {
expect(detectLanguage({ dependencies: { typescript: '3.8.0' } })).toBe(
SupportedLanguage.TYPESCRIPT_3_8
);
expect(detectLanguage({ dependencies: { typescript: '4.8.0' } })).toBe(
SupportedLanguage.TYPESCRIPT_3_8
);
});

it(`should return language typescript if the dependency is >TS4.9`, () => {
it(`should return language typescript-4-9 if the dependency is >TS4.9`, () => {
expect(detectLanguage({ dependencies: { typescript: '4.9.1' } })).toBe(
SupportedLanguage.TYPESCRIPT
SupportedLanguage.TYPESCRIPT_4_9
);
});

it(`should return language typescript if the dependency is =TS4.9`, () => {
expect(detectLanguage({ dependencies: { typescript: '4.9.0' } })).toBe(
SupportedLanguage.TYPESCRIPT
SupportedLanguage.TYPESCRIPT_4_9
);
});

it(`should return language typescript if the dependency is =TS4.9beta`, () => {
expect(detectLanguage({ dependencies: { typescript: '^4.9.0-beta' } })).toBe(
SupportedLanguage.TYPESCRIPT
SupportedLanguage.TYPESCRIPT_4_9
);
});

Expand Down
17 changes: 14 additions & 3 deletions code/lib/cli/src/detect.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import fs from 'fs';
import findUp from 'find-up';
import semver from 'semver';
import { logger } from '@storybook/node-logger';

import type { TemplateConfiguration, TemplateMatcher } from './project_types';
import {
Expand Down Expand Up @@ -177,9 +178,19 @@ export function detectLanguage(packageJson?: PackageJson) {
semver.gte(semver.coerce(version), '0.6.8')
))
) {
language = SupportedLanguage.TYPESCRIPT;
} else if (hasDependency(packageJson, 'typescript')) {
language = SupportedLanguage.TYPESCRIPT_LEGACY;
language = SupportedLanguage.TYPESCRIPT_4_9;
} else if (
hasDependency(packageJson, 'typescript', (version) =>
semver.gte(semver.coerce(version), '3.8.0')
)
) {
language = SupportedLanguage.TYPESCRIPT_3_8;
} else if (
hasDependency(packageJson, 'typescript', (version) =>
semver.lt(semver.coerce(version), '3.8.0')
)
) {
logger.warn('Detected TypeScript < 3.8, populating with JavaScript examples');
}

return language;
Expand Down
8 changes: 4 additions & 4 deletions code/lib/cli/src/generators/configure.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe('configureMain', () => {

test('should generate main.ts', async () => {
await configureMain({
language: SupportedLanguage.TYPESCRIPT,
language: SupportedLanguage.TYPESCRIPT_4_9,
addons: [],
storybookConfigFolder: '.storybook',
framework: {
Expand Down Expand Up @@ -141,7 +141,7 @@ describe('configurePreview', () => {

test('should generate preview.ts', async () => {
await configurePreview({
language: SupportedLanguage.TYPESCRIPT,
language: SupportedLanguage.TYPESCRIPT_4_9,
storybookConfigFolder: '.storybook',
});

Expand All @@ -168,15 +168,15 @@ describe('configurePreview', () => {
test('should not do anything if the framework template already included a preview', async () => {
(fse.pathExists as unknown as jest.Mock).mockReturnValueOnce(true);
await configurePreview({
language: SupportedLanguage.TYPESCRIPT,
language: SupportedLanguage.TYPESCRIPT_4_9,
storybookConfigFolder: '.storybook',
});
expect(fse.writeFile).not.toHaveBeenCalled();
});

test('should add prefix if frameworkParts are passed', async () => {
await configurePreview({
language: SupportedLanguage.TYPESCRIPT,
language: SupportedLanguage.TYPESCRIPT_4_9,
storybookConfigFolder: '.storybook',
frameworkPreviewParts: {
prefix: dedent`
Expand Down
6 changes: 3 additions & 3 deletions code/lib/cli/src/generators/configure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export async function configureMain({
};

const isTypescript =
language === SupportedLanguage.TYPESCRIPT || language === SupportedLanguage.TYPESCRIPT_LEGACY;
language === SupportedLanguage.TYPESCRIPT_4_9 || language === SupportedLanguage.TYPESCRIPT_3_8;

let mainConfigTemplate = dedent`<<import>>const config<<type>> = <<mainContents>>;
export default config;`;
Expand Down Expand Up @@ -96,8 +96,8 @@ export async function configureMain({
export async function configurePreview(options: ConfigurePreviewOptions) {
const { prefix = '' } = options.frameworkPreviewParts || {};
const isTypescript =
options.language === SupportedLanguage.TYPESCRIPT ||
options.language === SupportedLanguage.TYPESCRIPT_LEGACY;
options.language === SupportedLanguage.TYPESCRIPT_4_9 ||
options.language === SupportedLanguage.TYPESCRIPT_3_8;

const previewPath = `./${options.storybookConfigFolder}/preview.${isTypescript ? 'ts' : 'js'}`;

Expand Down
15 changes: 9 additions & 6 deletions code/lib/cli/src/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,15 @@ describe('Helpers', () => {
});

it.each`
language | exists | expected
${'javascript'} | ${['js', 'ts']} | ${'/js'}
${'typescript'} | ${['js', 'ts']} | ${'/ts'}
${'typescript'} | ${['js']} | ${'/js'}
${'javascript'} | ${[]} | ${''}
${'typescript'} | ${[]} | ${''}
language | exists | expected
${'javascript'} | ${['js', 'ts-4-9']} | ${'/js'}
${'typescript-4-9'} | ${['js', 'ts-4-9']} | ${'/ts-4-9'}
${'typescript-4-9'} | ${['js', 'ts-3-8']} | ${'/ts-3-8'}
${'typescript-3-8'} | ${['js', 'ts-3-8', 'ts-4-9']} | ${'/ts-3-8'}
${'typescript-3-8'} | ${['js', 'ts-4-9']} | ${'/js'}
${'typescript-4-9'} | ${['js']} | ${'/js'}
${'javascript'} | ${[]} | ${''}
${'typescript-4-9'} | ${[]} | ${''}
`(
`should copy $expected when folder $exists exists for language $language`,
async ({ language, exists, expected }) => {
Expand Down
21 changes: 9 additions & 12 deletions code/lib/cli/src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,33 +201,30 @@ export async function copyTemplateFiles({
}: CopyTemplateFilesOptions) {
const languageFolderMapping: Record<SupportedLanguage, string> = {
[SupportedLanguage.JAVASCRIPT]: 'js',
[SupportedLanguage.TYPESCRIPT]: 'ts',
[SupportedLanguage.TYPESCRIPT_LEGACY]: 'ts-legacy',
[SupportedLanguage.TYPESCRIPT_3_8]: 'ts-3-8',
[SupportedLanguage.TYPESCRIPT_4_9]: 'ts-4-9',
};
const templatePath = async () => {
const baseDir = getRendererDir(renderer);
const assetsDir = join(baseDir, 'template/cli');

const assetsLanguage = join(assetsDir, languageFolderMapping[language]);
const assetsJS = join(assetsDir, languageFolderMapping[SupportedLanguage.JAVASCRIPT]);
const assetsTSLegacy = join(
assetsDir,
languageFolderMapping[SupportedLanguage.TYPESCRIPT_LEGACY]
);
const assetsTS = join(assetsDir, languageFolderMapping[SupportedLanguage.TYPESCRIPT]);
const assetsTS38 = join(assetsDir, languageFolderMapping[SupportedLanguage.TYPESCRIPT_3_8]);

// Ideally use the assets that match the language & version.
if (await fse.pathExists(assetsLanguage)) {
return assetsLanguage;
}
if (language === SupportedLanguage.TYPESCRIPT && (await fse.pathExists(assetsTSLegacy))) {
return assetsTSLegacy;
}
if (language === SupportedLanguage.TYPESCRIPT_LEGACY && (await fse.pathExists(assetsTS))) {
return assetsTS;
// Use fallback typescript 3.8 assets if new ones aren't available
if (language === SupportedLanguage.TYPESCRIPT_4_9 && (await fse.pathExists(assetsTS38))) {
return assetsTS38;
}
// Fallback further to JS
if (await fse.pathExists(assetsJS)) {
return assetsJS;
}
// As a last resort, look for the root of the asset directory
if (await fse.pathExists(assetsDir)) {
return assetsDir;
}
Expand Down
4 changes: 2 additions & 2 deletions code/lib/cli/src/project_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ export type Builder = CoreBuilder | (string & {});

export enum SupportedLanguage {
JAVASCRIPT = 'javascript',
TYPESCRIPT_LEGACY = 'typescript-legacy',
TYPESCRIPT = 'typescript',
TYPESCRIPT_3_8 = 'typescript-3-8',
TYPESCRIPT_4_9 = 'typescript-4-9',
}

export type TemplateMatcher = {
Expand Down

0 comments on commit 3288a19

Please sign in to comment.