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

Core/CLI: Add extract function to PreviewWeb and use it in sb extract if available #17447

Merged
merged 3 commits into from Feb 10, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/cli/src/extract.ts
Expand Up @@ -12,7 +12,7 @@ const read = async (url: string) => {
await page.goto(url);

await page.waitForFunction(
'window.__STORYBOOK_STORY_STORE__ && window.__STORYBOOK_STORY_STORE__.extract && window.__STORYBOOK_STORY_STORE__.extract()'
'window.__STORYBOOK_PREVIEW__ && window.__STORYBOOK_PREVIEW__.extract && window.__STORYBOOK_PREVIEW__.extract()'
Copy link
Member

Choose a reason for hiding this comment

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

@tmeasday we should fall back to __STORYBOOK_STORY_STORE__ if window.__STORYBOOK_PREVIEW__.extract doesn't exist so that if user runs npx sb extract on an older version of SB, it doesn't error.

Copy link
Member Author

Choose a reason for hiding this comment

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

But aren't they by definition using the right version of SB?

Copy link
Member

Choose a reason for hiding this comment

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

No. extract runs puppeteer against a running storybook. That storybook could be any version AFAIK.

Meanwhile we encourage people to run npx sb so that they get the latest version of the CLI. So as soon as we release this as stable, npx sb will assume that the Storybook it's running against has the changes from this PR (unless the user has installed a specific version of the sb or @storybook/cli package, which we discourage). And that wouldn't be the case unless they've also upgraded their Storybook, which is a completely separate operation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh. OK. Will fix.

Copy link
Member

Choose a reason for hiding this comment

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

@shilman perhaps what we could do is add a test for sb extract targeting a bunch of chromatic storybook URLs, of various versions, this way we can assert we stay compatible?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds like a good idea @ndelangen. Would be good to do the same for Chromatic's version of extract also TBH. I'll get the set of uploaded SBs together.

Copy link
Member

Choose a reason for hiding this comment

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

I've updated the code for now and will let somebody follow up with extra tests

);
const data = JSON.parse(
await page.evaluate(async () => {
Expand Down
157 changes: 156 additions & 1 deletion lib/preview-web/src/PreviewWeb.test.ts
Expand Up @@ -4,8 +4,8 @@ import merge from 'lodash/merge';
import Events, { IGNORED_EXCEPTION } from '@storybook/core-events';
import { logger } from '@storybook/client-logger';
import addons, { mockChannel as createMockChannel } from '@storybook/addons';
import { ModuleImportFn } from '@storybook/store';
import { AnyFramework } from '@storybook/csf';
import type { ModuleImportFn } from '@storybook/store';

import { PreviewWeb } from './PreviewWeb';
import {
Expand Down Expand Up @@ -2725,4 +2725,159 @@ describe('PreviewWeb', () => {
);
});
});

describe('extract', () => {
// NOTE: if you are using storyStoreV6, and your `preview.js` throws, we do not currently
// detect it (as we do not wrap the import of `preview.js` in a `try/catch`). The net effect
// of that is that the `PreviewWeb`/`StoryStore` end up in an uninitalized state.
it('throws an error if the preview is uninitialized', async () => {
const preview = new PreviewWeb();
await expect(preview.extract()).rejects.toThrow(/Failed to initialize/);
});

it('throws an error if preview.js throws', async () => {
const err = new Error('meta error');
const preview = new PreviewWeb();
await expect(
preview.initialize({
importFn,
getProjectAnnotations: () => {
throw err;
},
})
).rejects.toThrow(err);

await expect(preview.extract()).rejects.toThrow(err);
});

it('shows an error if the stories.json endpoint 500s', async () => {
const err = new Error('sort error');
mockFetchResult = { status: 500, text: async () => err.toString() };

const preview = new PreviewWeb();
await expect(preview.initialize({ importFn, getProjectAnnotations })).rejects.toThrow(
'sort error'
);

await expect(preview.extract()).rejects.toThrow('sort error');
});

it('waits for stories to be cached', async () => {
const [gate, openGate] = createGate();

const gatedImportFn = async (path) => {
await gate;
return importFn(path);
};

const preview = await createAndRenderPreview({ importFn: gatedImportFn });

let extracted = false;
preview.extract().then(() => {
extracted = true;
});

expect(extracted).toBe(false);

openGate();
await new Promise((r) => setTimeout(r, 0)); // Let the promise resolve
expect(extracted).toBe(true);

expect(await preview.extract()).toMatchInlineSnapshot(`
Object {
"component-one--a": Object {
"argTypes": Object {
"foo": Object {
"name": "foo",
"type": Object {
"name": "string",
},
},
},
"args": Object {
"foo": "a",
},
"component": undefined,
"componentId": "component-one",
"id": "component-one--a",
"initialArgs": Object {
"foo": "a",
},
"kind": "Component One",
"name": "A",
"parameters": Object {
"__isArgsStory": false,
"docs": Object {
"container": [MockFunction],
},
"fileName": "./src/ComponentOne.stories.js",
},
"story": "A",
"subcomponents": undefined,
"title": "Component One",
},
"component-one--b": Object {
"argTypes": Object {
"foo": Object {
"name": "foo",
"type": Object {
"name": "string",
},
},
},
"args": Object {
"foo": "b",
},
"component": undefined,
"componentId": "component-one",
"id": "component-one--b",
"initialArgs": Object {
"foo": "b",
},
"kind": "Component One",
"name": "B",
"parameters": Object {
"__isArgsStory": false,
"docs": Object {
"container": [MockFunction],
},
"fileName": "./src/ComponentOne.stories.js",
},
"story": "B",
"subcomponents": undefined,
"title": "Component One",
},
"component-two--c": Object {
"argTypes": Object {
"foo": Object {
"name": "foo",
"type": Object {
"name": "string",
},
},
},
"args": Object {
"foo": "c",
},
"component": undefined,
"componentId": "component-two",
"id": "component-two--c",
"initialArgs": Object {
"foo": "c",
},
"kind": "Component Two",
"name": "C",
"parameters": Object {
"__isArgsStory": false,
"fileName": "./src/ComponentTwo.stories.js",
},
"playFunction": undefined,
"story": "C",
"subcomponents": undefined,
"title": "Component Two",
},
}
`);
});
});
});
29 changes: 29 additions & 0 deletions lib/preview-web/src/PreviewWeb.tsx
Expand Up @@ -80,6 +80,8 @@ export class PreviewWeb<TFramework extends AnyFramework> {

renderToDOM: WebProjectAnnotations<TFramework>['renderToDOM'];

previewEntryError?: Error;

previousSelection: Selection;

previousStory: Story<TFramework>;
Expand Down Expand Up @@ -294,6 +296,8 @@ export class PreviewWeb<TFramework extends AnyFramework> {
}: {
getProjectAnnotations: () => MaybePromise<ProjectAnnotations<TFramework>>;
}) {
delete this.previewEntryError;

const projectAnnotations = await this.getProjectAnnotationsOrRenderError(getProjectAnnotations);
if (!this.storyStore.projectAnnotations) {
await this.initializeWithProjectAnnotations(projectAnnotations);
Expand All @@ -306,6 +310,8 @@ export class PreviewWeb<TFramework extends AnyFramework> {
}

async onStoryIndexChanged() {
delete this.previewEntryError;

if (!this.storyStore.projectAnnotations) {
// We haven't successfully set project annotations yet,
// we need to do that before we can do anything else.
Expand Down Expand Up @@ -709,6 +715,28 @@ export class PreviewWeb<TFramework extends AnyFramework> {
};
}

// API
async extract(options?: { includeDocsOnly: boolean }) {
if (this.previewEntryError) {
throw this.previewEntryError;
}

if (!this.storyStore.projectAnnotations) {
// In v6 mode, if your preview.js throws, we never get a chance to initialize the preview
// or store, and the error is simply logged to the browser console. This is the best we can do
throw new Error(dedent`Failed to initialize Storybook.

Do you have an error in your \`preview.js\`? Check your Storybook's browser console for errors.`);
}

if (global.FEATURES?.storyStoreV7) {
await this.storyStore.cacheAllCSFFiles();
}

return this.storyStore.extract(options);
}

// UTILITIES
async cleanupPreviousRender({ unmountDocs = true }: { unmountDocs?: boolean } = {}) {
const previousViewMode = this.previousStory?.parameters?.docsOnly
? 'docs'
Expand All @@ -724,6 +752,7 @@ export class PreviewWeb<TFramework extends AnyFramework> {
}

renderPreviewEntryError(reason: string, err: Error) {
this.previewEntryError = err;
logger.error(reason);
logger.error(err);
this.view.showErrorDisplay(err);
Expand Down