TestBuild: Fix indexer bug

[ndelangen]

What I did

enable the --test flag internally when building our storybook

I discovered that the titlePrefix and directory fiels are super-important for a correct creation of the index.json.
... by flattening all the files without taking these into account, the conflicting stories-ids problem could occur, and the hierarchy of the index.json would be completely incorrect. (which would cause removed/renamed stories!).

I also am setting a different default_files_patterns now based on the disableMDXEntries setting, which should yield fewer individual pattern globs.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

• stories
• unit tests
• integration tests
• end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

• Add or update documentation reflecting your changes
• If you are deprecating/removing a feature, make sure to update
  MIGRATION.MD

Checklist for Maintainers

• When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts

• Make sure this PR contains one of the labels below:

  Available labels

  • bug: Internal changes that fixes incorrect behavior.
  • maintenance: User-facing maintenance tasks.
  • dependencies: Upgrading (sometimes downgrading) dependencies.
  • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
  • cleanup: Minor cleanup style change. Will not show up in release changelog.
  • documentation: Documentation only changes. Will not show up in release changelog.
  • feature request: Introducing a new feature.
  • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
  • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

[ndelangen]

This is the part I'm not really happy about... But it was the only solution I could really find without digging into this:

storybook/code/lib/core-common/src/utils/glob-to-regexp.ts

Lines 3 to 27 in ff6b28e

	export function globToRegexp(glob: string) {
	const regex = pico.makeRe(glob, {
	fastpaths: false,
	noglobstar: false,
	bash: false,
	});
	if (!regex.source.startsWith('^')) {
	throw new Error(`Invalid glob: >> ${glob} >> ${regex}`);
	}
	if (!glob.startsWith('./')) {
	return regex;
	}
	// makeRe is sort of funny. If you pass it a directory starting with `./` it
	// creates a matcher that expects files with no prefix (e.g. `src/file.js`)
	// but if you pass it a directory that starts with `../` it expects files that
	// start with `../`. Let's make it consistent.
	// Globs starting `**` require special treatment due to the regex they
	// produce, specifically a negative look-ahead
	return new RegExp(
	['^\\.', glob.startsWith('./**') ? '' : '[\\\\/]', regex.source.substring(1)].join('')
	);
	}

... and it really do not want to go digging into that code, right now.

I'm thinking this is "close enough" for now.

[tmeasday]

It would be a lot easier to understand what this code was doing if we had a test for it.

I am assuming it turns something like:

[{ directory: 'foo', files: '**/*.(mdx|stories.js)' }]

Into something like:

[
  { directory: 'foo', files: '**/first-story.stories.js' },
  { directory: 'foo', files: '**/second-story.stories.js' },
  //..
]

Where that list can be very long? That seems like it would have a pretty detritmental impact on build times if that list was really long, but I guess we would have to test it it out to know for sure.

I wonder if there's a way to hit the most common cases and just drop the mdx| bit from the regexp, if we see a common pattern like '**/*.@(mdx|stories.@(js|jsx|mjs|ts|tsx))';

Also, what if the expanded list is empty?

[ndelangen]

Needed because of:

storybook/code/ui/.storybook/preview.tsx

Lines 141 to 159 in ab7fd6d

	(Story, { loaded: { docsContext } }) =>
	docsContext ? (
	<DocsContext.Provider value={docsContext}>
	<Story />
	</DocsContext.Provider>
	) : (
	<Story />
	),
	/**
	* This decorator adds wrappers that contains global styles for stories to be targeted by.
	* Activated with parameters.docsStyles = true
	*/ (Story, { parameters: { docsStyles } }) =>
	docsStyles ? (
	<DocsPageWrapper>
	<Story />
	</DocsPageWrapper>
	) : (
	<Story />
	),

This breaks at runtime, because if blocks is replaced as a module by an empty object, that object (obviously) does not contain what is needed here.

[kasperpeulen]

LGTM, just one comment

[kasperpeulen]

Isn't using a flatMap a lot simpler?

[tmeasday]

Why snake case when the other options are camel case?

[tmeasday]

It would be a lot easier to understand what this code was doing if we had a test for it.

I am assuming it turns something like:

[{ directory: 'foo', files: '**/*.(mdx|stories.js)' }]

Into something like:

[
  { directory: 'foo', files: '**/first-story.stories.js' },
  { directory: 'foo', files: '**/second-story.stories.js' },
  //..
]

Where that list can be very long? That seems like it would have a pretty detritmental impact on build times if that list was really long, but I guess we would have to test it it out to know for sure.

I wonder if there's a way to hit the most common cases and just drop the mdx| bit from the regexp, if we see a common pattern like '**/*.@(mdx|stories.@(js|jsx|mjs|ts|tsx))';

Also, what if the expanded list is empty?