Improve error reporting for duplicate story definitions #19094

Niznikr · 2022-09-02T13:51:28Z

Describe the bug
This is for a pnpm monorepo project migrating from 7.0.0-alpha.10 webpack React (which works perfectly) to 7.0.0-alpha.28 with the Vite framework. I get the following error when storyStoreV7 is true:

The console shows:

Error: Duplicate stories with id: components-icon--default
    at StoryIndexGenerator.chooseDuplicate (/Users/robertniznik/code/launchdarkly/launchpad-ui/node_modules/.pnpm/@[email protected]_szel5ggy6oiqgqxwc4a6yc2j34/node_modules/@storybook/core-server/dist/cjs/utils/StoryIndexGenerator.js:351:44)
    at /Users/robertniznik/code/launchdarkly/launchpad-ui/node_modules/.pnpm/@[email protected]_szel5ggy6oiqgqxwc4a6yc2j34/node_modules/@storybook/core-server/dist/cjs/utils/StoryIndexGenerator.js:390:36
    at Array.forEach (<anonymous>)
    at StoryIndexGenerator.sortStories (/Users/robertniznik/code/launchdarkly/launchpad-ui/node_modules/.pnpm/@[email protected]_szel5ggy6oiqgqxwc4a6yc2j34/node_modules/@storybook/core-server/dist/cjs/utils/StoryIndexGenerator.js:386:17)
    at StoryIndexGenerator.getIndex (/Users/robertniznik/code/launchdarkly/launchpad-ui/node_modules/.pnpm/@[email protected]_szel5ggy6oiqgqxwc4a6yc2j34/node_modules/@storybook/core-server/dist/cjs/utils/StoryIndexGenerator.js:415:29)
    at async /Users/robertniznik/code/launchdarkly/launchpad-ui/node_modules/.pnpm/@[email protected]_szel5ggy6oiqgqxwc4a6yc2j34/node_modules/@storybook/core-server/dist/cjs/dev-server.js:119:24

I validated that the same error appears for @storybook/react-webpack5 past version 7.0.0-alpha.10. Not sure how to proceed as nothing was changed besides bumping the alpha version.

System
System:
OS: macOS 12.5.1
CPU: (10) arm64 Apple M1 Pro
Binaries:
Node: 16.15.0 - /usr/local/bin/node
Yarn: 1.22.17 - /opt/homebrew/bin/yarn
npm: 8.5.5 - /usr/local/bin/npm
Browsers:
Chrome: 105.0.5195.52
Firefox: 104.0.1
Safari: 15.6.1
npmPackages:
@storybook/addon-a11y: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/addon-actions: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/addon-backgrounds: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/addon-docs: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/addon-essentials: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/addon-highlight: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/addon-interactions: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/addon-measure: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/addon-outline: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/addons: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/api: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/channel-postmessage: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/channel-websocket: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/cli: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/client-api: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/client-logger: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/components: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/core-client: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/core-events: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/preview-web: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/react: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/react-vite: ^7.0.0-alpha.28 => 7.0.0-alpha.28
@storybook/testing-library: ^0.0.13 => 0.0.13
@storybook/theming: ^7.0.0-alpha.28 => 7.0.0-alpha.28

Additional context
Stories are written in CSF3.

The text was updated successfully, but these errors were encountered:

IanVS · 2022-09-05T01:39:52Z

@Niznikr thanks for the report. Is there any chance you can share a link to the project, or create a minimal reproduction? Also, have you confirmed that you don't have multiple stories files with the same title?

Niznikr · 2022-09-05T13:22:33Z

Hi @IanVS! I will try to get a minimal reproduction for you tomorrow. I have confirmed that I don't have multiple stories with the same title. It even happens when I update the regex to target a single story.

Niznikr · 2022-09-06T14:05:43Z

So I was able to re-enable storyStoreV7 by using a stricter custom regex for the stories:

const fg = require('fast-glob');

const getStories = () =>
  fg.sync([path.resolve(__dirname, `../packages/**/stories/*.stories.tsx`), '!**/node_modules']);

module.exports = {
  stories: async () => [...getStories()],

It seems past 7.0.0-alpha.10 Storybook was also capturing the stories of linked packages in the pnpm workspace. For example, our modal package depends on the button package. It gets linked and included in the modal's node_modules so technically the button's stories are in scope to get pulled in. That explains the duplicate stories error and why a stricter regex fixes things.

IanVS · 2022-09-06T14:14:20Z

@Niznikr what was your glob before the change?

@shilman is it expected that stories globs match inside node_modules?

Niznikr · 2022-09-06T14:24:00Z

It was '../packages/**/*.stories.@(js|jsx|ts|tsx)' which had worked for prior versions.

Niznikr · 2022-09-06T14:25:13Z

To be clear, it seemed to match the individual node_modules of each package, not the root node_modules.

shilman · 2022-09-06T23:41:51Z

@IanVS usually stories globs matching inside node_modules means that they are not "tight" enough, which can have some performance costs in webpack. also published components' docgen often fails, since many packages only publish the dist directory (or equivalent). so we don't recommend it, but I also don't think we explicitly forbid it. we could consider that, though, if it helps?

IanVS · 2022-09-07T01:11:44Z

Thanks @shilman. It sounds like maybe something changed in recent alpha versions, though, according to @Niznikr. They said it was not matching stories in node_modules until recently. They updated storybook, and started to get failures. @Niznikr if you can find the exact alpha version where this started, we could probably trace down the reason and figure out whether it was intentional or not.

Also BTW, I'm in Ann Arbor as well! Maybe we'll run into each other sometime. ;)

Niznikr · 2022-09-07T01:55:08Z

Sure @IanVS I can narrow down where it started. So cool to hear you're in the area! I'm usually at a RoosRoast, Hyperion, or some other coffee shop in the morning :P Would be fun to chat sometime.

Niznikr · 2022-09-07T14:48:09Z

It started in 7.0.0-alpha.12

IanVS · 2022-09-07T15:34:28Z

I don't see anything jumping out to me in https://github.com/storybookjs/storybook/releases/tag/v7.0.0-alpha.12 that would indicate this is an intentional change. @tmeasday it looks like most of those are your PRs. Does it make sense that stories in node_modules would start being included in globs in this release?

tmeasday · 2022-09-07T23:44:39Z

That release includes the PRs that include the error that is being thrown here, but AFAIK nothing that would influence the files that are processed by the indexer. So I'd hazard a guess that maybe the file was being processed before but it just didn't lead to an error? Couple questions @Niznikr:

Am I understanding correctly that the same file is matching more than once due to symlinks? (So the same file appears to be at different paths from SB's perspective?)
It's unclear if you know the files in node_modules weren't being matched before, or just it wasn't erroring, when you say "had worked for prior versions." -- my guess it's the latter, but I think maybe helpful to know for sure.

If I am correct about the above, I am not sure we would consider it a bug that you aren't allowed to have two "different" files that define the same story. Arguably allowing one to clobber the other was the real bug 🤷

Niznikr · 2022-09-08T00:10:10Z

Correct it is a result of symlinks via pnpm workspaces
Yeah I only know that starting in alpha.12 I get the TypeError: Failed to fetch error and using the custom glob that ignores node_modules fixes things

tmeasday · 2022-09-08T01:04:57Z

@shilman WDYT about this new behaviour? Do we consider it a bug? Do we need to document it?

yuri-scarbaci-lenio · 2022-11-23T16:38:41Z

We are afected by this too and the stricter regex isn't helping us out,
It's not clear to me yet how do I find the duplicate-id file paths since the error message isn't providing any reference that I seem to be able to understand

We are at alpha.52

 @storybook/cli v7.0.0-alpha.52

using webpack builder config
info => Loading presets
info => Loading presets
info => Loading presets
info => Starting manager..
Error: Duplicate stories with id: hooks--use-focus-trap
   at StoryIndexGenerator.chooseDuplicate ([/local/folder/path]/node_modules/.pnpm/@[email protected]_7g5yyhfnhuwo76jyxbdrfo5ybm/node_modules/@storybook/core-server/dist/cjs/utils/StoryIndexGenerator.js:302:44)
   at [/local/folder/path]/node_modules/.pnpm/@[email protected]_7g5yyhfnhuwo76jyxbdrfo5ybm/node_modules/@storybook/core-server/dist/cjs/utils/StoryIndexGenerator.js:335:36
   at Array.forEach (<anonymous>)
   at StoryIndexGenerator.sortStories ([/local/folder/path]/node_modules/.pnpm/@[email protected]_7g5yyhfnhuwo76jyxbdrfo5ybm/node_modules/@storybook/core-server/dist/cjs/utils/StoryIndexGenerator.js:332:17)
   at StoryIndexGenerator.getIndex ([/local/folder/path]/node_modules/.pnpm/@[email protected]_7g5yyhfnhuwo76jyxbdrfo5ybm/node_modules/@storybook/core-server/dist/cjs/utils/StoryIndexGenerator.js:360:29)
   at async [/local/folder/path]/node_modules/.pnpm/@[email protected]_7g5yyhfnhuwo76jyxbdrfo5ybm/node_modules/@storybook/core-server/dist/cjs/dev-server.js:83:24

"devDependencies": {
   "@babel/runtime": "~7.17.9",
   "@storybook/addon-a11y": "7.0.0-alpha.52",
   "@storybook/addon-essentials": "7.0.0-alpha.52",
   "@storybook/addon-events": "~6.3.0-alpha.21",
   "@storybook/addon-interactions": "7.0.0-alpha.52",
   "@storybook/addon-links": "7.0.0-alpha.52",
   "@storybook/addon-storysource": "7.0.0-alpha.52",
   "@storybook/builder-webpack5": "7.0.0-alpha.52",
   "@storybook/manager-webpack5": "7.0.0-alpha.10",
   "@storybook/react": "7.0.0-alpha.52",
   "@storybook/react-webpack5": "7.0.0-alpha.0",
   "@storybook/theming": "7.0.0-alpha.52",
   "storybook": "next"
 }

(in our configuration we don't use most of the addons, we may have to clean up a bunch of the dependencies ASAP)

we are also using pnpm (and we are moving to v7 alpha because 6.5 suddendly broke-up and by reading the issues around we found out that v7 should be solving most of the pnpm related issues)

I will try to keep on debugging on this, but any leads or help on how I should go around the debugging of this would be extremely appreciated

IanVS · 2022-11-23T16:45:08Z

@yuri-scarbaci-lenio, are you using a similar project structure to #19094 (comment), where stories from linked packages are being included?

If not, I would suggest searching through your stories for two of them using the same title of hooks.

Maybe you can describe your project structure a bit?

yuri-scarbaci-lenio · 2022-11-23T16:48:54Z

Actually I just found out I did have duplicated stories,

for me to be able to debug this I had to modify the node_modules file to get some meaningful information

Can we add the information on the error thrown by StoryIndexGenerator.js -> chooseDuplicate
at

    // This shouldn't be possible, but double check and use for typing
    if (worseEntry.type === 'story') throw new Error(`Duplicate stories with id: ${firstEntry.id}`);

can we either include all the firstEntry and secondEntry object or at the very least the .importPath of both the entries?

tmeasday · 2022-11-24T00:11:50Z

@yuri-scarbaci-lenio would you be interested in submitting a PR doing that? I'd be very happy to review it. Otherwise I'll get to this eventually as part of the 7.0 release cycle.

yuri-scarbaci-lenio · 2022-11-30T21:17:55Z

@tmeasday sure, see related PR at #20038

sneko · 2022-12-20T10:14:08Z

Using pnpm too and got the duplication issue, thanks to #19094 (comment) for pointing excluding node_modules was required.

It should be documented if there is no will to prevent this (the question is maybe, is it too specific to pnpm...).

shilman · 2022-12-26T09:18:05Z

@tmeasday sorry for the slow reply (notifications hell!). i think the console error makes sense Error: Duplicate stories with id: hooks--use-focus-trap and it would be great if we could also show that in the UI. I believe @ndelangen was looking into propagating node errors into the browser as part of another issue, so maybe it also can apply here.

ndelangen · 2023-01-05T14:11:50Z

Here's an idea:
When the StoryIndexGenerator is doing it's work, and encounters a problem like this, it writes this information in a problems array.

The consumer of the JSON could then do something with that array of problems.. storybook could display the problems in it's UI somewhere.

We can't send an event on some channel, because the browser doesn't open / start listening to the channel yet, until the StoryIndexGenerator is completed.

yuri-scarbaci-lenio · 2023-01-10T14:40:47Z

My 2 cents as per
#20038 (comment)

in general I think that a project as big as storybook should have a dedicated error handling layer that allows for extra information in a non string format...

Whatever you choose to do with this particular problem, I think this solution should be an extension of a more abstract and generic layer, the "error handling" layer

Based on what it's stated above I think the layer should be at the very least be capable of handling different type of errors,

static/compile errors ( such would be the case for this issue )
on the fly errors ( errors that occurs during user browser navigation )

And such layer should provide different configuration regarding the output of such catched errors, the bare minimum imho would be:

CLI output
Browser console output

I personally would put the

rendered in the browser UI output ( basically the react equivalent of <ErrorBoundry> )

as a nice to have

If this was to be planned correctly, sharing the errors would be a problem reduced to "sharing the error layer", which should be a much simpler problem to solve given that storybook developer mode start is actually mounting a pseudo-server...
(I know, I don't propose much of a practical solution, just a really high level idea, but one gotta start somewhere right?)

tmeasday · 2023-01-11T03:25:51Z

@yuri-scarbaci-lenio you are scratching the surface of an issue that's been discussed elsewhere (that there are like a dozen different types of errors you can trigger in SB and we don't really handle them consistently at all). I'm not sure for 7.0 we are likely to resolve that.

However, if we stick to just these errors on static analysis and index generation, then there are a few concerns:

Attaching useful info to the errors we find
Collecting multiple errors rather than just showing the first
(Potentially) showing indexing errors in the browser somehow.

I would suggest that (3) is out of scope for 7.0, and would probably be considered as part of the first discussion. But 1&2 are totally in scope and I think we should rationalize it alongside #20044

yuri-scarbaci-lenio · 2023-01-11T17:16:11Z

I see,
well to me that's extremely good news,
goes to show that such an important topic was not missed by storybook team,
so I would say, kudos & thanks for sharing!

Do you have a RFC or some publicly available centralised discussion regarding the error handling layer?
or you plan to create one eventually? (even if it's not going to be in scope for 7.0)

Niznikr added bug needs triage labels Sep 2, 2022

IanVS added the builder-vite label Sep 5, 2022

yuri-scarbaci-lenio mentioned this issue Nov 23, 2022

[Bug]: "storybook-config-entry.js doesn't exist" - building Storybook 7.0.0-alpha.52 & Angular 14 Component Library #19940

Closed

tmeasday added linear and removed needs triage labels Nov 24, 2022

tmeasday changed the title ~~SB 7.0.0-alpha.28 storyStoreV7 shows TypeError: Failed to fetch~~ Improve error reporting for duplicate story definitions Nov 24, 2022

YuriScarbaci mentioned this issue Nov 30, 2022

Core: added .entries property to error thrown when duplicate stories are present #20038

Merged

3 tasks

tmeasday added this to Core Team Projects Jan 11, 2023

tmeasday mentioned this issue Jan 11, 2023

Core: Show "booting" progress until story is specified or errors #20425

Merged

This was referenced Jan 15, 2023

Fix: Make index failures result in build failures #20326

Closed

[Feature Request]: Rationalize index generation errors #20663

Closed

shilman removed this from Core Team Projects Feb 10, 2023

ndelangen removed the linear-ghaction label Feb 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve error reporting for duplicate story definitions #19094

Improve error reporting for duplicate story definitions #19094

Niznikr commented Sep 2, 2022 •

edited

Loading

IanVS commented Sep 5, 2022

Niznikr commented Sep 5, 2022

Niznikr commented Sep 6, 2022 •

edited

Loading

IanVS commented Sep 6, 2022

Niznikr commented Sep 6, 2022

Niznikr commented Sep 6, 2022

shilman commented Sep 6, 2022

IanVS commented Sep 7, 2022

Niznikr commented Sep 7, 2022

Niznikr commented Sep 7, 2022

IanVS commented Sep 7, 2022

tmeasday commented Sep 7, 2022 •

edited

Loading

Niznikr commented Sep 8, 2022

tmeasday commented Sep 8, 2022

yuri-scarbaci-lenio commented Nov 23, 2022 •

edited by IanVS

Loading

IanVS commented Nov 23, 2022

yuri-scarbaci-lenio commented Nov 23, 2022 •

edited

Loading

tmeasday commented Nov 24, 2022

yuri-scarbaci-lenio commented Nov 30, 2022

sneko commented Dec 20, 2022 •

edited

Loading

shilman commented Dec 26, 2022

ndelangen commented Jan 5, 2023

yuri-scarbaci-lenio commented Jan 10, 2023 •

edited

Loading

tmeasday commented Jan 11, 2023

yuri-scarbaci-lenio commented Jan 11, 2023

Improve error reporting for duplicate story definitions #19094

Improve error reporting for duplicate story definitions #19094

Comments

Niznikr commented Sep 2, 2022 • edited Loading

IanVS commented Sep 5, 2022

Niznikr commented Sep 5, 2022

Niznikr commented Sep 6, 2022 • edited Loading

IanVS commented Sep 6, 2022

Niznikr commented Sep 6, 2022

Niznikr commented Sep 6, 2022

shilman commented Sep 6, 2022

IanVS commented Sep 7, 2022

Niznikr commented Sep 7, 2022

Niznikr commented Sep 7, 2022

IanVS commented Sep 7, 2022

tmeasday commented Sep 7, 2022 • edited Loading

Niznikr commented Sep 8, 2022

tmeasday commented Sep 8, 2022

yuri-scarbaci-lenio commented Nov 23, 2022 • edited by IanVS Loading

IanVS commented Nov 23, 2022

yuri-scarbaci-lenio commented Nov 23, 2022 • edited Loading

tmeasday commented Nov 24, 2022

yuri-scarbaci-lenio commented Nov 30, 2022

sneko commented Dec 20, 2022 • edited Loading

shilman commented Dec 26, 2022

ndelangen commented Jan 5, 2023

yuri-scarbaci-lenio commented Jan 10, 2023 • edited Loading

tmeasday commented Jan 11, 2023

yuri-scarbaci-lenio commented Jan 11, 2023

Niznikr commented Sep 2, 2022 •

edited

Loading

Niznikr commented Sep 6, 2022 •

edited

Loading

tmeasday commented Sep 7, 2022 •

edited

Loading

yuri-scarbaci-lenio commented Nov 23, 2022 •

edited by IanVS

Loading

yuri-scarbaci-lenio commented Nov 23, 2022 •

edited

Loading

sneko commented Dec 20, 2022 •

edited

Loading

yuri-scarbaci-lenio commented Jan 10, 2023 •

edited

Loading