Merge pull request #21 from j3rem1e/warning-duplicate-id

Fix duplicated exported id
storybookjs · Jun 9, 2021 · 984e9d6 · 984e9d6
2 parents bcb3cbd + 59274fe
commit 984e9d6
Show file tree

Hide file tree

Showing 9 changed files with 220 additions and 54 deletions.
diff --git a/src/parser/collect-stories.js b/src/parser/collect-stories.js
@@ -21,7 +21,7 @@ const createFragment = document.createDocumentFragment
   ? () => document.createDocumentFragment()
   : () => document.createElement('div');
 
-export default (StoriesComponent, stories) => {
+export default (StoriesComponent, { stories = {}, allocatedIds }) => {
   const repositories = {
     meta: null,
     stories: [],
@@ -72,7 +72,7 @@ export default (StoriesComponent, stories) => {
       .reduce((all, story) => {
         const { id, name, template, component, source = false, ...props } = story;
 
-        const storyId = extractId(story);
+        const storyId = extractId(story, allocatedIds);
         if (!storyId) {
           return all;
         }

diff --git a/src/parser/collect-stories.test.js b/src/parser/collect-stories.test.js
@@ -3,7 +3,7 @@ import TestStories from '../components/__tests__/TestStories.svelte';
 
 describe('parse-stories', () => {
   test('Extract Stories', () => {
-    const data = collectStories(TestStories, { 'tpl:tpl2': 'tpl2src' });
+    const data = collectStories(TestStories, { stories: { 'tpl:tpl2': 'tpl2src' } });
     const { stories, meta } = data;
     expect(meta).toMatchInlineSnapshot(`
       Object {

diff --git a/src/parser/extract-id.test.js b/src/parser/extract-id.test.js
@@ -4,4 +4,7 @@ describe('extract-id', () => {
   test('name with spaces', () => {
     expect(extractId({ name: 'Name with spaces' })).toBe('NameWithSpaces');
   });
+  test('duplicates id', () => {
+    expect(extractId({ name: 'Button' }, ['Button'])).toBe('Button77471352');
+  });
 });
diff --git a/src/parser/extract-id.ts b/src/parser/extract-id.ts
@@ -1,8 +1,31 @@
+import { logger } from '@storybook/client-logger';
+
 // extract a story id
-export function extractId({ id, name }: { id?: string; name?: string }): string {
+export function extractId(
+  {
+    id,
+    name,
+  }: {
+    id?: string;
+    name?: string;
+  },
+  allocatedIds: string[] = []
+): string {
   if (id) {
     return id;
   }
 
-  return name.replace(/\W+(.)/g, (_, chr) => chr.toUpperCase());
+  let generated = name.replace(/\W+(.)/g, (_, chr) => chr.toUpperCase());
+  if (allocatedIds.indexOf(generated) >= 0) {
+    logger.warn(`Story name conflict with exports - Please add an explicit id for story ${name}`);
+    generated += hashCode(name);
+  }
+  return generated;
+}
+
+function hashCode(str: string): string {
+  return str
+    .split('')
+    .reduce((prevHash, currVal) => ((prevHash << 5) - prevHash + currVal.charCodeAt(0)) | 0, 0) // eslint-disable-line
+    .toString(16);
 }
diff --git a/src/parser/extract-stories.test.js b/src/parser/extract-stories.test.js
@@ -14,11 +14,17 @@ describe('extractSource', () => {
         `)
     ).toMatchInlineSnapshot(`
       Object {
-        "MyStory": Object {
-          "hasArgs": false,
-          "name": "MyStory",
-          "source": "<div>a story</div>",
-          "template": false,
+        "allocatedIds": Array [
+          "default",
+          "Story",
+        ],
+        "stories": Object {
+          "MyStory": Object {
+            "hasArgs": false,
+            "name": "MyStory",
+            "source": "<div>a story</div>",
+            "template": false,
+          },
         },
       }
     `);
@@ -36,11 +42,17 @@ describe('extractSource', () => {
         `)
     ).toMatchInlineSnapshot(`
       Object {
-        "myId": Object {
-          "hasArgs": false,
-          "name": "MyStory",
-          "source": "<div>a story</div>",
-          "template": false,
+        "allocatedIds": Array [
+          "default",
+          "Story",
+        ],
+        "stories": Object {
+          "myId": Object {
+            "hasArgs": false,
+            "name": "MyStory",
+            "source": "<div>a story</div>",
+            "template": false,
+          },
         },
       }
     `);
@@ -58,11 +70,17 @@ describe('extractSource', () => {
         `)
     ).toMatchInlineSnapshot(`
       Object {
-        "MyStory": Object {
-          "hasArgs": true,
-          "name": "MyStory",
-          "source": "<div>a story</div>",
-          "template": false,
+        "allocatedIds": Array [
+          "default",
+          "Story",
+        ],
+        "stories": Object {
+          "MyStory": Object {
+            "hasArgs": true,
+            "name": "MyStory",
+            "source": "<div>a story</div>",
+            "template": false,
+          },
         },
       }
     `);
@@ -80,11 +98,17 @@ describe('extractSource', () => {
         `)
     ).toMatchInlineSnapshot(`
       Object {
-        "tpl:MyTemplate": Object {
-          "hasArgs": false,
-          "name": "MyTemplate",
-          "source": "<div>a template</div>",
-          "template": true,
+        "allocatedIds": Array [
+          "default",
+          "Template",
+        ],
+        "stories": Object {
+          "tpl:MyTemplate": Object {
+            "hasArgs": false,
+            "name": "MyTemplate",
+            "source": "<div>a template</div>",
+            "template": true,
+          },
         },
       }
     `);
@@ -102,11 +126,17 @@ describe('extractSource', () => {
         `)
     ).toMatchInlineSnapshot(`
       Object {
-        "tpl:default": Object {
-          "hasArgs": false,
-          "name": "default",
-          "source": "<div>a template</div>",
-          "template": true,
+        "allocatedIds": Array [
+          "default",
+          "Template",
+        ],
+        "stories": Object {
+          "tpl:default": Object {
+            "hasArgs": false,
+            "name": "default",
+            "source": "<div>a template</div>",
+            "template": true,
+          },
         },
       }
     `);
@@ -127,17 +157,23 @@ describe('extractSource', () => {
         `)
     ).toMatchInlineSnapshot(`
       Object {
-        "Story1": Object {
-          "hasArgs": false,
-          "name": "Story1",
-          "source": "<div>story 1</div>",
-          "template": false,
-        },
-        "Story2": Object {
-          "hasArgs": false,
-          "name": "Story2",
-          "source": "<div>story 2</div>",
-          "template": false,
+        "allocatedIds": Array [
+          "default",
+          "Template",
+        ],
+        "stories": Object {
+          "Story1": Object {
+            "hasArgs": false,
+            "name": "Story1",
+            "source": "<div>story 1</div>",
+            "template": false,
+          },
+          "Story2": Object {
+            "hasArgs": false,
+            "name": "Story2",
+            "source": "<div>story 2</div>",
+            "template": false,
+          },
         },
       }
     `);
@@ -157,11 +193,48 @@ describe('extractSource', () => {
         `)
     ).toMatchInlineSnapshot(`
       Object {
-        "Story1": Object {
-          "hasArgs": false,
-          "name": "Story1",
-          "source": "<div>story 1</div>",
-          "template": false,
+        "allocatedIds": Array [
+          "default",
+          "SBStory",
+          "SBMeta",
+        ],
+        "stories": Object {
+          "Story1": Object {
+            "hasArgs": false,
+            "name": "Story1",
+            "source": "<div>story 1</div>",
+            "template": false,
+          },
+        },
+      }
+    `);
+  });
+  test('Duplicate Id', () => {
+    expect(
+      extractStories(`
+        <script>
+          import { Story } from '@storybook/svelte';
+          import Button from './Button.svelte';
+        </script>
+
+        <Story name="Button">
+          <div>a story</div>
+        </Story>
+        `)
+    ).toMatchInlineSnapshot(`
+      Object {
+        "allocatedIds": Array [
+          "default",
+          "Story",
+          "Button",
+        ],
+        "stories": Object {
+          "Button77471352": Object {
+            "hasArgs": false,
+            "name": "Button",
+            "source": "<div>a story</div>",
+            "template": false,
+          },
         },
       }
     `);

diff --git a/src/parser/extract-stories.ts b/src/parser/extract-stories.ts
@@ -9,6 +9,11 @@ interface StoryDef {
   hasArgs: boolean;
 }
 
+interface StoriesDef {
+  stories: Record<string, StoryDef>;
+  allocatedIds: string[];
+}
+
 function getStaticAttribute(name: string, node: any): string {
   // extract the attribute
   const attribute = node.attributes.find(
@@ -33,10 +38,12 @@ function getStaticAttribute(name: string, node: any): string {
  * @param component Component Source
  * @returns Map of storyName -> source
  */
-export function extractStories(component: string): Record<string, StoryDef> {
+export function extractStories(component: string): StoriesDef {
   // compile
   const { ast } = svelte.compile(component);
 
+  const allocatedIds: string[] = ['default'];
+
   const localNames = {
     Story: 'Story',
     Template: 'Template',
@@ -58,6 +65,18 @@ export function extractStories(component: string): Record<string, StoryDef> {
     },
   });
 
+  // extracts allocated Ids
+  svelte.walk(ast.instance, {
+    enter(node: any) {
+      if (node.type === 'ImportDeclaration') {
+        node.specifiers
+          .map((n: any) => n.local.name)
+          .forEach((name: string) => allocatedIds.push(name));
+        this.skip();
+      }
+    },
+  });
+
   const stories: Record<string, StoryDef> = {};
   svelte.walk(ast.html, {
     enter(node: any) {
@@ -77,10 +96,13 @@ export function extractStories(component: string): Record<string, StoryDef> {
           name = 'default';
         }
 
-        const id = extractId({
-          id: getStaticAttribute('id', node),
-          name,
-        });
+        const id = extractId(
+          {
+            id: getStaticAttribute('id', node),
+            name,
+          },
+          isTemplate ? undefined : allocatedIds
+        );
 
         if (name && id) {
           // ignore stories without children
@@ -103,5 +125,8 @@ export function extractStories(component: string): Record<string, StoryDef> {
     },
   });
 
-  return stories;
+  return {
+    stories,
+    allocatedIds,
+  };
 }
diff --git a/src/parser/svelte-stories-loader.ts b/src/parser/svelte-stories-loader.ts
@@ -44,7 +44,8 @@ function transformSvelteStories(code: string) {
 
   const source = readFileSync(resource).toString();
 
-  const stories = extractStories(source);
+  const storiesDef = extractStories(source);
+  const { stories } = storiesDef;
 
   const storyDef = Object.entries(stories)
     .filter(([, def]) => !def.template)
@@ -55,7 +56,7 @@ function transformSvelteStories(code: string) {
 
   return dedent`${codeWithoutDefaultExport}
     const { default: parser } = require('${parser}');
-    const __storiesMetaData = parser(${componentName}, ${JSON.stringify(stories)});
+    const __storiesMetaData = parser(${componentName}, ${JSON.stringify(storiesDef)});
     export default __storiesMetaData.meta;
     ${storyDef};
   `;