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

Migrate addon storyshots to TS #7674

Merged
merged 25 commits into from
Oct 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
76b672e
chore: emit TS declaration file during `addon-storyshots` build
gaetanmaisse Aug 4, 2019
a22f903
refactor: move files of base and api part of `addon-storyshots` to TS
gaetanmaisse Aug 4, 2019
017729c
refactor: create StoryshotsOptions to export options available for ad…
gaetanmaisse Aug 4, 2019
f1fd558
refactor: move addon-storyshots framework base file to TS and types them
gaetanmaisse Aug 4, 2019
83e9b5f
feat: make frameworkerLoader load both JS and TS loaders as they will…
gaetanmaisse Aug 6, 2019
892ab2c
refactor: move angular loader in addon-storyshots framework part to TS
gaetanmaisse Aug 4, 2019
1169a74
refactor: move html loader of addon-storyshots framework part to TS
gaetanmaisse Aug 5, 2019
001315f
refactor: move preact loader of addon-storyshots framework part to TS
gaetanmaisse Aug 5, 2019
3f782db
refactor: move react loader of addon-storyshots framework part to TS
gaetanmaisse Aug 6, 2019
54a7e9b
refactor: move riot loader of addon-storyshots framework part to TS
gaetanmaisse Aug 6, 2019
aff52bb
refactor: move react native loader of addon-storyshots framework part…
gaetanmaisse Aug 6, 2019
a8262f9
refactor: rename 'rn' folder to 'react-native'
gaetanmaisse Aug 6, 2019
5a7bb14
refactor: move svelte loader of addon-storyshots framework part to TS
gaetanmaisse Aug 6, 2019
ce76410
refactor: move vue loader of addon-storyshots framework part to TS
gaetanmaisse Aug 6, 2019
a1680b9
refactor: extract SupportedFramework and use it to improve typings of…
gaetanmaisse Aug 6, 2019
6127e38
refactor: used names export instead of default one for Stories2SnapsC…
gaetanmaisse Aug 6, 2019
bc8f5bd
refactor: use `reduce` instead of `flatMap` because it is not availab…
gaetanmaisse Aug 7, 2019
f821891
refactor: improve typings of some base classes of storyshots-core
gaetanmaisse Aug 8, 2019
8cbabb5
chore: add tsconfig in `storyshots-puppeteer`
gaetanmaisse Aug 8, 2019
5352afb
refactor: move all files of addon-storyshots-puppeteer to TS
gaetanmaisse Aug 8, 2019
7dc87d7
refactor: improve typings of addon-storyshots-puppeteer
gaetanmaisse Aug 8, 2019
2f13904
doc: fix `storyshots-puppeteer` README, so of the examples were not a…
gaetanmaisse Aug 8, 2019
de01450
refactor: improve storyshot-core typings
gaetanmaisse Oct 5, 2019
16170f2
fix: fix `react-native` string used in framework check of imageSnapshot
gaetanmaisse Oct 5, 2019
aad29ed
feat: use `puppeteer-core` instead of `puppeteer`
gaetanmaisse Oct 5, 2019
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
5 changes: 5 additions & 0 deletions addons/storyshots/storyshots-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"README.md"
],
"main": "dist/index.js",
"types": "dist/index.d.ts",
"scripts": {
"build-storybook": "build-storybook",
"example": "jest storyshot.test",
Expand All @@ -31,6 +32,10 @@
"dependencies": {
"@jest/transform": "^24.9.0",
"@storybook/addons": "5.3.0-alpha.13",
"@storybook/client-api": "5.3.0-alpha.13",
"@types/glob": "^7.1.1",
"@types/jest": "^24.0.16",
"@types/jest-specific-snapshot": "^0.5.3",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@emilio-martinez @kroeder @ndelangen what's the best practice about @types in SB codebase? I added them in dependencies because some of the types I exported in this addon are based on jest (and other libs) ones. So if a SB user want to use SB type it should install @types/jest too... 🤷‍♂

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll talk about this later today, but I think the preliminary agreement is to add the types of a regular dependency also as a regular dependency.

"core-js": "^3.0.1",
"glob": "^7.1.3",
"global": "^4.3.2",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import Stories2SnapsConverter from './Stories2SnapsConverter';
import { Stories2SnapsConverter } from './Stories2SnapsConverter';

const target = new Stories2SnapsConverter();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
import path from 'path';
import dedent from 'ts-dedent';

const defaultOptions = {
const defaultOptions: Stories2SnapsConverterOptions = {
snapshotsDirName: '__snapshots__',
snapshotExtension: '.storyshot',
storiesExtensions: ['.js', '.jsx', '.ts', '.tsx'],
};

class DefaultStories2SnapsConverter {
constructor(options = {}) {
export interface Stories2SnapsConverterOptions {
storiesExtensions: string[];
snapshotExtension: string;
snapshotsDirName: string;
}

export class Stories2SnapsConverter {
options: Stories2SnapsConverterOptions;

constructor(options: Partial<Stories2SnapsConverterOptions> = {}) {
this.options = {
...defaultOptions,
...options,
Expand All @@ -17,14 +25,14 @@ class DefaultStories2SnapsConverter {

getSnapshotExtension = () => this.options.snapshotExtension;

getStoryshotFile(fileName) {
getStoryshotFile(fileName: string) {
const { dir, name } = path.parse(fileName);
const { snapshotsDirName, snapshotExtension } = this.options;

return path.format({ dir: path.join(dir, snapshotsDirName), name, ext: snapshotExtension });
}

getSnapshotFileName(context) {
getSnapshotFileName(context: { fileName?: string, kind: any }) {
const { fileName, kind } = context;

if (!fileName) {
Expand All @@ -45,7 +53,7 @@ class DefaultStories2SnapsConverter {
return this.getStoryshotFile(fileName);
}

getPossibleStoriesFiles(storyshotFile) {
getPossibleStoriesFiles(storyshotFile: string) {
const { dir, name } = path.parse(storyshotFile);
const { storiesExtensions } = this.options;

Expand All @@ -58,5 +66,3 @@ class DefaultStories2SnapsConverter {
);
}
}

export default DefaultStories2SnapsConverter;
23 changes: 23 additions & 0 deletions addons/storyshots/storyshots-core/src/api/StoryshotsOptions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { IOptions } from 'glob';
import { Stories2SnapsConverter } from '../Stories2SnapsConverter';
import { SupportedFramework } from '../frameworks';
import { RenderTree } from '../frameworks/Loader';

export interface StoryshotsOptions {
asyncJest?: boolean;
config?: (options: any) => void;
integrityOptions?: IOptions | false;
configPath?: string;
suite?: string;
storyKindRegex?: RegExp | string;
storyNameRegex?: RegExp | string;
framework?: SupportedFramework;
test?: (story: any, context: any, renderTree: RenderTree, options?: any) => any;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ndelangen @kroeder are there already existing types somewhere for story and context?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, in lib/addons

renderer?: Function;
snapshotSerializers?: jest.SnapshotSerializerPlugin[];
/**
* @Deprecated The functionality of this option is completely covered by snapshotSerializers which should be used instead.
*/
serializer?: any;
stories2snapsConverter?: Stories2SnapsConverter;
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { snapshotWithOptions } from '../test-bodies';
import Stories2SnapsConverter from '../Stories2SnapsConverter';
import { Stories2SnapsConverter } from '../Stories2SnapsConverter';
import { StoryshotsOptions } from './StoryshotsOptions';

const ignore = ['**/node_modules/**'];
const defaultStories2SnapsConverter = new Stories2SnapsConverter();

function getIntegrityOptions({ integrityOptions }) {
function getIntegrityOptions({ integrityOptions }: StoryshotsOptions) {
if (integrityOptions === false) {
return false;
}
Expand All @@ -13,14 +14,18 @@ function getIntegrityOptions({ integrityOptions }) {
return false;
}

const ignoreOption: string[] = Array.isArray(integrityOptions.ignore)
? integrityOptions.ignore
: [];

return {
...integrityOptions,
ignore: [...ignore, ...(integrityOptions.ignore || [])],
ignore: [...ignore, ...ignoreOption],
absolute: true,
};
}

function ensureOptionsDefaults(options) {
function ensureOptionsDefaults(options: StoryshotsOptions) {
const {
suite = 'Storyshots',
asyncJest,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,27 @@ import ensureOptionsDefaults from './ensureOptionsDefaults';
import snapshotsTests from './snapshotsTestsTemplate';
import integrityTest from './integrityTestTemplate';
import loadFramework from '../frameworks/frameworkLoader';
import { StoryshotsOptions } from './StoryshotsOptions';

global.STORYBOOK_REACT_CLASSES = global.STORYBOOK_REACT_CLASSES || {};

const methods = ['beforeAll', 'beforeEach', 'afterEach', 'afterAll'];
type TestMethod = 'beforeAll' | 'beforeEach' | 'afterEach' | 'afterAll';
gaetanmaisse marked this conversation as resolved.
Show resolved Hide resolved
const methods: TestMethod[] = ['beforeAll', 'beforeEach', 'afterEach', 'afterAll'];

function callTestMethodGlobals(testMethod) {
function callTestMethodGlobals(
testMethod: { [key in TestMethod]?: Function } & { [key in string]: any }
) {
methods.forEach(method => {
if (typeof testMethod[method] === 'function') {
global[method](testMethod[method]);
}
});
}

const isDisabled = parameter => parameter === false || (parameter && parameter.disable === true);
const isDisabled = (parameter: any) =>
parameter === false || (parameter && parameter.disable === true);

function testStorySnapshots(options = {}) {
function testStorySnapshots(options: StoryshotsOptions = {}) {
if (typeof describe !== 'function') {
throw new Error('testStorySnapshots is intended only to be used inside jest');
}
Expand Down Expand Up @@ -47,23 +52,29 @@ function testStorySnapshots(options = {}) {
.raw()
.filter(({ name }) => (storyNameRegex ? name.match(storyNameRegex) : true))
.filter(({ kind }) => (storyKindRegex ? kind.match(storyKindRegex) : true))
.reduce((acc, item) => {
const { kind, storyFn: render, parameters } = item;
const existing = acc.find(i => i.kind === kind);
const { fileName } = item.parameters;
.reduce(
(acc, item) => {
const { kind, storyFn: render, parameters } = item;
const existing = acc.find((i: any) => i.kind === kind);
const { fileName } = item.parameters;

if (!isDisabled(parameters.storyshots)) {
if (existing) {
existing.children.push({ ...item, render, fileName });
} else {
acc.push({
kind,
children: [{ ...item, render, fileName }],
});
if (!isDisabled(parameters.storyshots)) {
if (existing) {
existing.children.push({ ...item, render, fileName });
} else {
acc.push({
kind,
children: [{ ...item, render, fileName }],
});
}
}
}
return acc;
}, []);
return acc;
},
[] as Array<{
kind: string;
children: any[];
}>
);

if (data.length) {
callTestMethodGlobals(testMethod);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import fs from 'fs';
import glob from 'glob';
import { describe, it } from 'global';

function integrityTest(integrityOptions, stories2snapsConverter) {
function integrityTest(integrityOptions: any, stories2snapsConverter: any) {
if (integrityOptions === false) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { describe, it } from 'global';
import { addSerializer } from 'jest-specific-snapshot';

function snapshotTest({ item, asyncJest, framework, testMethod, testMethodParams }) {
function snapshotTest({ item, asyncJest, framework, testMethod, testMethodParams }: any) {
const { name } = item;
const context = { ...item, framework };

if (asyncJest === true) {
it(name, done =>
it(name, (done: jest.DoneCallback) =>
testMethod({
done,
story: item,
Expand All @@ -25,28 +25,28 @@ function snapshotTest({ item, asyncJest, framework, testMethod, testMethodParams
}
}

function snapshotTestSuite({ item, suite, ...restParams }) {
function snapshotTestSuite({ item, suite, ...restParams }: any) {
const { kind, children } = item;
// eslint-disable-next-line jest/valid-describe
describe(suite, () => {
// eslint-disable-next-line jest/valid-describe
describe(kind, () => {
children.forEach(c => {
children.forEach((c: any) => {
snapshotTest({ item: c, ...restParams });
});
});
});
}

function snapshotsTests({ data, snapshotSerializers, ...restParams }) {
function snapshotsTests({ data, snapshotSerializers, ...restParams }: any) {
if (snapshotSerializers) {
snapshotSerializers.forEach(serializer => {
snapshotSerializers.forEach((serializer: any) => {
addSerializer(serializer);
expect.addSnapshotSerializer(serializer);
});
}

data.forEach(item => {
data.forEach((item: any) => {
snapshotTestSuite({ item, ...restParams });
});
}
Expand Down
17 changes: 17 additions & 0 deletions addons/storyshots/storyshots-core/src/frameworks/Loader.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { ClientApi } from '@storybook/client-api';
import { StoryshotsOptions } from '../api/StoryshotsOptions';
import { SupportedFramework } from './SupportedFramework';

export type RenderTree = (story: any, context?: any, options?: any) => any;

export interface Loader {
load: (
options: StoryshotsOptions
) => {
framework: SupportedFramework;
renderTree: RenderTree;
renderShallowTree: any;
storybook: ClientApi;
};
test: (options: StoryshotsOptions) => boolean;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export type SupportedFramework =
| 'angular'
| 'html'
| 'preact'
| 'react'
| 'riot'
| 'react-native'
| 'svelte'
| 'vue';
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import 'core-js';
import 'core-js/es/reflect';
import hasDependency from '../hasDependency';
import configure from '../configure';
import { Loader } from '../Loader';
import { StoryshotsOptions } from '../../api/StoryshotsOptions';

function setupAngularJestPreset() {
// Needed to prevent "Zone.js has detected that ZoneAwarePromise `(window|global).Promise` has been overwritten."
Expand All @@ -18,13 +20,13 @@ function setupAngularJestPreset() {
require.requireActual('jest-preset-angular/setupJest');
}

function test(options) {
function test(options: StoryshotsOptions): boolean {
return (
options.framework === 'angular' || (!options.framework && hasDependency('@storybook/angular'))
);
}

function load(options) {
function load(options: StoryshotsOptions) {
setupAngularJestPreset();

const { configPath, config } = options;
Expand All @@ -33,7 +35,7 @@ function load(options) {
configure({ configPath, config, storybook });

return {
framework: 'angular',
framework: 'angular' as const,
renderTree: require.requireActual('./renderTree').default,
renderShallowTree: () => {
throw new Error('Shallow renderer is not supported for angular');
Expand All @@ -42,7 +44,9 @@ function load(options) {
};
}

export default {
const angularLoader: Loader = {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the way I found to have type-check according to Loader class, if anyone has a better proposition?

load,
test,
};

export default angularLoader;
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,21 @@ import { initModuleData } from './helpers';
addSerializer(HTMLCommentSerializer);
addSerializer(AngularSnapshotSerializer);

function getRenderedTree(story) {
function getRenderedTree(story: any) {
const currentStory = story.render();

const { moduleMeta, AppComponent } = initModuleData(currentStory);

TestBed.configureTestingModule({
imports: [...moduleMeta.imports],
declarations: [...moduleMeta.declarations],
providers: [...moduleMeta.providers],
schemas: [NO_ERRORS_SCHEMA, ...moduleMeta.schemas],
bootstrap: [...moduleMeta.bootstrap],
});
TestBed.configureTestingModule(
// TODO: take a look at `bootstrap` because it looks it does not exists in TestModuleMetadata
{
imports: [...moduleMeta.imports],
declarations: [...moduleMeta.declarations],
providers: [...moduleMeta.providers],
schemas: [NO_ERRORS_SCHEMA, ...moduleMeta.schemas],
bootstrap: [...moduleMeta.bootstrap],
} as any
);

TestBed.overrideModule(BrowserDynamicTestingModule, {
set: {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import fs from 'fs';
import path from 'path';

function getConfigPathParts(configPath) {
function getConfigPathParts(configPath: string): string {
const resolvedConfigPath = path.resolve(configPath);

if (fs.lstatSync(resolvedConfigPath).isDirectory()) {
Expand All @@ -11,7 +11,7 @@ function getConfigPathParts(configPath) {
return resolvedConfigPath;
}

function configure(options) {
function configure(options: { configPath?: string; config: any; storybook: any }): void {
const { configPath = '.storybook', config, storybook } = options;

if (config && typeof config === 'function') {
Expand Down
Loading