Support configuration object for stories

[kazuma1989]

Issues

Fixes #77.

Hi, maintainers and contributors!

After #172, I found another issue relating Storybook 6.4.
As of Storybook 6.4, stories can be an array of StoriesSpecifier object, but the builder can't handle it.

The StoriesSpecifier is defined and used below.
https://github.com/storybookjs/storybook/blob/80410528e8911ec4e1708899aec96ae8196d829e/lib/core-common/src/types.ts#L377-L382
https://github.com/storybookjs/storybook/blob/80410528e8911ec4e1708899aec96ae8196d829e/lib/core-common/src/types.ts#L257-L274

module.exports = {
  stories: [
   { directory: '../src', files: '*.story.tsx', titlePrefix: 'foo' }
  ]
};

The code above should be OK but will raise an error saying:

18:13:28 [vite] Internal server error: The "path" argument must be of type string. Received an instance of Object
      at validateString (internal/validators.js:124:11)
      at Object.isAbsolute (path.js:1029:5)
      at /Users/kazuma/work/oss/storybook-builder-vite/packages/storybook-builder-vite/codegen-importfn-script.js:55:31
      at Array.map (<anonymous>)
      at generateImportFnScriptCode (/Users/kazuma/work/oss/storybook-builder-vite/packages/storybook-builder-vite/codegen-importfn-script.js:54:19)
      at runMicrotasks (<anonymous>)
      at processTicksAndRejections (internal/process/task_queues.js:93:5)
      at async Object.load (/Users/kazuma/work/oss/storybook-builder-vite/node_modules/vite/dist/node/chunks/dep-7e125991.js:42779:32)
      at async doTransform (/Users/kazuma/work/oss/storybook-builder-vite/node_modules/vite/dist/node/chunks/dep-7e125991.js:57417:24)

Reproduction

You can see it by checking out the commit 9fed56c1e321abf7e4089fb4c9fe78b1460b5092 and running the following command.

cd packages/example-workspaces
yarn run storybook

Resolution

Using normalizeStories() imported from @storybook/core-common has resolved the issue.

The implementation is based on the one in @storybook/builder-webpack5.

https://github.com/storybookjs/storybook/blob/80410528e8911ec4e1708899aec96ae8196d829e/lib/builder-webpack5/src/preview/iframe-webpack.config.ts#L92-L95

I added the @storybook/core-common package as a peer dependency.
All the Storybook's base packages (@storybook/react, @storybook/svelte, @storybook/vue and so on) are depends on the package.
So it will be OK, I think.

FIX

normalizeStories is needed to have more control over auto-title-generation, introduced in Storybook 6.4.

cf. https://storybook.js.org/blog/component-story-format-3-0/

Therefore, normalizeStories is not necessary for users of version 6.3 who only specify an array of strings for stories.

Removed @storybook/core-common from peer dependencies.
And now use normalizeStories only when it is available, and use simple logic to match types when it is not.
Both version 6.3 and 6.4 will work.

Thanks.

[eirslett]

Looks OK to me! Why were the story titles removed? To test the recent title-auto-generation feature of Storybook?

[kazuma1989]

To test the recent title-auto-generation feature of Storybook?

That's right. 😃
The reason why I specified an array of objects for stories is because I wanted to use titlePrefix for the title-auto-generation.

[eirslett]

@IanVS @joshwooding what do you think, should we put this out there as version 0.2.0?

[IanVS]

I rebased this on master, and found that there's now another use of await options.presets.apply('stories', [], options) in optimizeDeps. CI passes because it doesn't use optimizeDeps.

This presents a challenge, because we expected the stories to be globs, which we could send straight to vite to treat as entries. With this change, that's not possible, and we'd have to send the full list of files. Which I guess is fine? I'll try it out and see how it works.

[IanVS]

OK, I pushed up a fix for that issue, but identified another. This PR only works when storyStoreV7 is enabled. I don't have a chance to look into why right now, @joshwooding maybe something in codegen-modern-iframe-script.js can be ported over to codegen-iframe-script.js?

[IanVS]

This needs to work without storyStoreV7 enabled.

As another datapoint, I found earlier today that automatic titles only works when this flag is enabled as well.

[joshwooding]

This needs to work without storyStoreV7 enabled.

As another datapoint, I found earlier today that automatic titles only works when this flag is enabled as well.

Pretty sure we're waiting on #76 for automatic titles without storyStoreV7

[IanVS]

If that's the purpose of #76, so far it's not working. I'm not completely clear what the goal of that PR is though. Anyway, I'll let @kazuma1989 decide whether this is worth removing my last commit that removes storyStoreV7 and merging this. Personally it seems like we shouldn't release a 0.2.0 until we get automatic title generation working without storyStoreV7, although this PR is definitely a necessary step towards getting there.

Requested changes are unrelated to this PR

[joshwooding]

If that's the purpose of #76, so far it's not working. I'm not completely clear what the goal of that PR is though. Anyway, I'll let @kazuma1989 decide whether this is worth removing my last commit that removes storyStoreV7 and merging this. Personally it seems like we shouldn't release a 0.2.0 until we get automatic title generation working without storyStoreV7, although this PR is definitely a necessary step towards getting there.

I think you’re right, we should aim to get #76 merged before releasing 0.2.0

[IanVS]

OK, I dropped the commit which had removed storyStoreV7. I think we can probably merge this, and then figure out what's going on there with the rest of auto-title generation.

[joshwooding]

@IanVS @eirslett @kazuma1989 I've bumped the @storybook/* packages to 6.4 here since we require the peerDep to be >=6.4.0

[IanVS]

It's definitely not working correctly, I made some notes on what I've found so far: #201.

[IanVS]

@joshwooding @eirslett Since we're moving from storybook-builder-vite to @storybook/builder-vite, do you think it's a good time to clean this up and merge/publish as 0.2.0?

[joshwooding]

@IanVS Makes sense to me 👍🏽

[eirslett]

Makes sense to me too!

[IanVS]

Hah, so turns out we already only support sb 6.4+, since #218 (0.1.17). Oops.

Since we haven't heard any complaints, I propose we just make this official (put it in the README), and if anyone needs 6.3 support we can point them to 0.1.16. Then, we can merge this and release without a breaking semver change. WDYT?

[joshwooding]

Hah, so turns out we already only support sb 6.4+, since #218 (0.1.17). Oops.

Since we haven't heard any complaints, I propose we just make this official (put it in the README), and if anyone needs 6.3 support we can point them to 0.1.16. Then, we can merge this and release without a breaking semver change. WDYT?

Makes sense to me. I've rebased the PR to help.

[IanVS]

Just curious, why 6.4.3 for the @storybook/* dependencies?

[joshwooding]

Just curious, why 6.4.3 for the @storybook/* dependencies?

This PR was to match the peerDependency but not sure why it was chosen there.

[IanVS]

Strange, it looks like it was added in the PR that converted us to typescript. https://github.com/storybookjs/builder-vite/pull/195/files#diff-7bcd234238498627a82d47315d21db88a6433dd4eb978c99c8397387e1e7d0caR30

@mrauhu do you recall why this version was added to peerDependencies, instead of say, 6.4.0?

At any rate, let's get this released, and we can change the ranges later if we decide to.

[IanVS]

Thanks @kazuma1989 for your work here, and for your patience!

[mrauhu]

Strange, it looks like it was added in the PR that converted us to typescript. https://github.com/storybookjs/builder-vite/pull/195/files#diff-7bcd234238498627a82d47315d21db88a6433dd4eb978c99c8397387e1e7d0caR30

@mrauhu do you recall why this version was added to peerDependencies, instead of say, 6.4.0?

@IanVS because this version based on the real version value in yarn.lock:

builder-vite/yarn.lock

Lines 2997 to 2999 in 1d1360e

	"@storybook/core-common@npm:6.4.3":
	version: 6.4.3
	resolution: "@storybook/core-common@npm:6.4.3"