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

Support storybook preview file #792

Merged
merged 16 commits into from
May 17, 2023
Merged

Support storybook preview file #792

merged 16 commits into from
May 17, 2023

Conversation

askoufis
Copy link
Contributor

@askoufis askoufis commented May 10, 2023

With the deprecation (and future removal) of the storiesOf API, the best place for global configuration of stories is the preview.js file. See the storybook docs for more info about what this file can be used for.

This PR adds support for storybook's preview.(js|ts|tsx) file. When running sku storybook or sku build-storybook, if a preview file is detected in the .storybook folder relative to the sku config folder, a symlink will be created in sku's storybook config directory that points to the preview file.

I also removed the decorator that sku was injecting into every story. This was required in order to render text correctly, back when we were still using basekick. It was kept when we moved to capsize, but isn't actually needed. Not marking this as a breaking change as it won't affect any projects other than Braid, and even then it will only result in some screenshot diffs with a few border pixels missing.

I've also optimized the storybook test utilities so we can re-use the same iframe that puppeteer gives us between tests. We test the same story in each test anyway. Previously, each storybook test was opening the page, clicking a button, getting the story iframe, and then running a selector on the iframe. This resulted in each test taking about 1-2s (on my machine). Now that we're re-using the same iframe between tests, each test takes a few milliseconds.

EDIT: Looks like a similar test speedup on CI too 🏎️

@askoufis askoufis requested a review from a team as a code owner May 10, 2023 06:21
@changeset-bot
Copy link

changeset-bot bot commented May 10, 2023

🦋 Changeset detected

Latest commit: 98de0e9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
sku Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@askoufis
Copy link
Contributor Author

So, because we're now making/cleaning up symlinks when starting/building storybook, when running multiple storybooks at once, which happens when running sku's tests, there's a possibility that a symlink could be created by one instance but cleaned up by another, so storybook throws an error because it can't resolve the file.

I think the best solution to this would be to move all storybook tests to a single fixture, probably storybook-config, so we only ever run a single instance of storybook at once (hopefully).

@askoufis
Copy link
Contributor Author

askoufis commented May 11, 2023

Blocked pending #793.

Copy link
Contributor

Choose a reason for hiding this comment

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

👋

);

expect(text).toEqual('Braid Text decorator');
expect(fontSize).toEqual('16px');
Copy link
Contributor

Choose a reason for hiding this comment

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

one day.... maybe one day.... but should be fine for the foreseeable future.

Copy link
Contributor

@mrm007 mrm007 left a comment

Choose a reason for hiding this comment

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

Nicely done 🍒 🐎

.changeset/chilled-planes-report.md Outdated Show resolved Hide resolved
.changeset/large-needles-serve.md Outdated Show resolved Hide resolved
docs/docs/storybook.md Outdated Show resolved Hide resolved
packages/sku/lib/storybook.js Show resolved Hide resolved
packages/sku/lib/storybook.js Outdated Show resolved Hide resolved
packages/sku/lib/storybook.js Outdated Show resolved Hide resolved
packages/sku/lib/storybook.js Outdated Show resolved Hide resolved
* If 1 file is found, a symlink is created to it from the provided storybook config directory.
* Does nothing if 0 or >1 files are found.
*
* @param {string} storybookConfigDirectory The path to the storybook config directory
Copy link
Contributor

Choose a reason for hiding this comment

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

I find your lack of respect for the Storybook brand name disturbing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sToRyBoOk

@askoufis askoufis enabled auto-merge (squash) May 17, 2023 06:11
@askoufis askoufis merged commit b7b5918 into master May 17, 2023
2 checks passed
@askoufis askoufis deleted the storybook-previewjs branch May 17, 2023 06:20
@seek-oss-ci seek-oss-ci mentioned this pull request May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants