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

Docs: Fix issue with referencing non-story files with names similar or equal to docs files #21348

Merged
merged 6 commits into from
Mar 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 32 additions & 5 deletions code/lib/core-server/src/utils/StoryIndexGenerator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,17 @@ describe('StoryIndexGenerator', () => {
"title": "A",
"type": "story",
},
"componentreference--docs": Object {
"id": "componentreference--docs",
"importPath": "./src/docs2/ComponentReference.mdx",
"name": "docs",
"storiesImports": Array [],
"tags": Array [
"docs",
],
"title": "ComponentReference",
"type": "docs",
},
"docs2-yabbadabbadooo--docs": Object {
"id": "docs2-yabbadabbadooo--docs",
"importPath": "./src/docs2/Title.mdx",
Expand Down Expand Up @@ -802,6 +813,7 @@ describe('StoryIndexGenerator', () => {
.toMatchInlineSnapshot(`
Array [
"A",
"titlePrefix/ComponentReference",
"A",
"titlePrefix/NoTitle",
"A",
Expand Down Expand Up @@ -860,6 +872,17 @@ describe('StoryIndexGenerator', () => {
"title": "A",
"type": "story",
},
"componentreference--info": Object {
"id": "componentreference--info",
"importPath": "./src/docs2/ComponentReference.mdx",
"name": "Info",
"storiesImports": Array [],
"tags": Array [
"docs",
],
"title": "ComponentReference",
"type": "docs",
},
"docs2-yabbadabbadooo--info": Object {
"id": "docs2-yabbadabbadooo--info",
"importPath": "./src/docs2/Title.mdx",
Expand Down Expand Up @@ -918,6 +941,7 @@ describe('StoryIndexGenerator', () => {
expect(Object.keys((await generator.getIndex()).entries)).toMatchInlineSnapshot(`
Array [
"a--story-one",
"componentreference--docs",
"a--metaof",
"notitle--docs",
"a--second-docs",
Expand Down Expand Up @@ -947,6 +971,7 @@ describe('StoryIndexGenerator', () => {
expect(Object.keys((await generator.getIndex()).entries)).toMatchInlineSnapshot(`
Array [
"a--story-one",
"componentreference--docs",
"a--metaof",
"notitle--docs",
"a--second-docs",
Expand Down Expand Up @@ -978,6 +1003,7 @@ describe('StoryIndexGenerator', () => {
expect(Object.keys((await generator.getIndex()).entries)).toMatchInlineSnapshot(`
Array [
"a--story-one",
"componentreference--story-one",
"a--metaof",
"notitle--story-one",
"a--second-docs",
Expand Down Expand Up @@ -1037,6 +1063,7 @@ describe('StoryIndexGenerator', () => {
"a--second-docs",
"a--story-one",
"second-nested-g--story-one",
"componentreference--docs",
"notitle--docs",
"first-nested-deeply-f--story-one",
]
Expand Down Expand Up @@ -1076,7 +1103,7 @@ describe('StoryIndexGenerator', () => {
const generator = new StoryIndexGenerator([storiesSpecifier, docsSpecifier], options);
await generator.initialize();
await generator.getIndex();
expect(toId).toHaveBeenCalledTimes(5);
expect(toId).toHaveBeenCalledTimes(6);

toIdMock.mockClear();
await generator.getIndex();
Expand Down Expand Up @@ -1135,7 +1162,7 @@ describe('StoryIndexGenerator', () => {
const generator = new StoryIndexGenerator([storiesSpecifier, docsSpecifier], options);
await generator.initialize();
await generator.getIndex();
expect(toId).toHaveBeenCalledTimes(5);
expect(toId).toHaveBeenCalledTimes(6);

generator.invalidate(docsSpecifier, './src/docs2/Title.mdx', false);

Expand All @@ -1157,7 +1184,7 @@ describe('StoryIndexGenerator', () => {
const generator = new StoryIndexGenerator([storiesSpecifier, docsSpecifier], options);
await generator.initialize();
await generator.getIndex();
expect(toId).toHaveBeenCalledTimes(5);
expect(toId).toHaveBeenCalledTimes(6);

generator.invalidate(storiesSpecifier, './src/A.stories.js', false);

Expand Down Expand Up @@ -1257,7 +1284,7 @@ describe('StoryIndexGenerator', () => {
const generator = new StoryIndexGenerator([docsSpecifier, storiesSpecifier], options);
await generator.initialize();
await generator.getIndex();
expect(toId).toHaveBeenCalledTimes(5);
expect(toId).toHaveBeenCalledTimes(6);

expect(Object.keys((await generator.getIndex()).entries)).toContain('notitle--docs');

Expand All @@ -1279,7 +1306,7 @@ describe('StoryIndexGenerator', () => {
const generator = new StoryIndexGenerator([docsSpecifier, storiesSpecifier], options);
await generator.initialize();
await generator.getIndex();
expect(toId).toHaveBeenCalledTimes(5);
expect(toId).toHaveBeenCalledTimes(6);

expect(Object.keys((await generator.getIndex()).entries)).toContain('a--metaof');

Expand Down
40 changes: 15 additions & 25 deletions code/lib/core-server/src/utils/StoryIndexGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,31 +212,21 @@ export class StoryIndexGenerator {
}

findDependencies(absoluteImports: Path[]) {
const dependencies = [] as StoriesCacheEntry[];
const foundImports = new Set();
this.specifierToCache.forEach((cache) => {
const fileNames = Object.keys(cache).filter((fileName) => {
const foundImport = absoluteImports.find((storyImport) => fileName.startsWith(storyImport));
if (foundImport) foundImports.add(foundImport);
return !!foundImport;
});
fileNames.forEach((fileName) => {
const cacheEntry = cache[fileName];
if (cacheEntry && cacheEntry.type === 'stories') {
dependencies.push(cacheEntry);
} else {
throw new Error(`Unexpected dependency: ${cacheEntry}`);
}
});
});

// imports can include non-story imports, so it's ok if
// there are fewer foundImports than absoluteImports
// if (absoluteImports.length !== foundImports.size) {
// throw new Error(`Missing dependencies: ${absoluteImports.filter((p) => !foundImports.has(p))}`));
// }

return dependencies;
return [...this.specifierToCache.values()].flatMap((cache: SpecifierStoriesCache) =>
Object.entries(cache)
.filter(([fileName, cacheEntry]) => {
// We are only interested in stories cache entries (and assume they've been processed already)
// If we found a match in the cache that's still null or not a stories file,
// it is a docs file and it isn't a dependency / storiesImport.
// See https://github.com/storybookjs/storybook/issues/20958
if (!cacheEntry || cacheEntry.type !== 'stories') return false;

return !!absoluteImports.find((storyImport) =>
fileName.match(new RegExp(`^${storyImport}(\\.[^.]+)?$`))
);
})
.map(([_, cacheEntry]) => cacheEntry as StoriesCacheEntry)
);
}

async extractStories(specifier: NormalizedStoriesSpecifier, absolutePath: Path) {
Expand Down
2 changes: 2 additions & 0 deletions code/lib/core-server/src/utils/__mockdata__/src/A.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// This could be the component for `A.stories.js`
export const A = {};
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// This a component that happens to have the same name as an MDX file referencing it
export const ComponentReference = {};
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{/* This will overlap with ComponentReference.mdx (this file). There's not much we can do about this */}
import ComponentReference from './ComponentReference';

{/* This prefixes A.stories.ts, we need to make sure we don't get confused. */}
import { A } from '../A';
29 changes: 29 additions & 0 deletions code/lib/core-server/src/utils/stories-json.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,17 @@ describe('useStoriesJson', () => {
"title": "D",
"type": "story",
},
"docs2-componentreference--docs": Object {
"id": "docs2-componentreference--docs",
"importPath": "./src/docs2/ComponentReference.mdx",
"name": "docs",
"storiesImports": Array [],
"tags": Array [
"docs",
],
"title": "docs2/ComponentReference",
"type": "docs",
},
"docs2-notitle--docs": Object {
"id": "docs2-notitle--docs",
"importPath": "./src/docs2/NoTitle.mdx",
Expand Down Expand Up @@ -367,6 +378,23 @@ describe('useStoriesJson', () => {
],
"title": "D",
},
"docs2-componentreference--docs": Object {
"id": "docs2-componentreference--docs",
"importPath": "./src/docs2/ComponentReference.mdx",
"kind": "docs2/ComponentReference",
"name": "docs",
"parameters": Object {
"__id": "docs2-componentreference--docs",
"docsOnly": true,
"fileName": "./src/docs2/ComponentReference.mdx",
},
"storiesImports": Array [],
"story": "docs",
"tags": Array [
"docs",
],
"title": "docs2/ComponentReference",
},
"docs2-notitle--docs": Object {
"id": "docs2-notitle--docs",
"importPath": "./src/docs2/NoTitle.mdx",
Expand Down Expand Up @@ -655,6 +683,7 @@ describe('useStoriesJson', () => {
expect(send).toHaveBeenCalledTimes(1);
expect(send.mock.calls[0][0]).toMatchInlineSnapshot(`
"Unable to index files:
- ./src/docs2/ComponentReference.mdx: You cannot use \`.mdx\` files without using \`storyStoreV7\`.
- ./src/docs2/MetaOf.mdx: You cannot use \`.mdx\` files without using \`storyStoreV7\`.
- ./src/docs2/NoTitle.mdx: You cannot use \`.mdx\` files without using \`storyStoreV7\`.
- ./src/docs2/SecondMetaOf.mdx: You cannot use \`.mdx\` files without using \`storyStoreV7\`.
Expand Down