Skip to content

Commit

Permalink
Merge pull request #246 from storybookjs/fix-invalid-identifiers
Browse files Browse the repository at this point in the history
fix: properly transform invalid identifiers
  • Loading branch information
JReinhold authored Dec 19, 2024
2 parents 93e6758 + 8959105 commit f96d9f9
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 4 deletions.
22 changes: 22 additions & 0 deletions src/compiler/pre-transform/codemods/legacy-story.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,28 @@ describe(transformLegacyStory.name, () => {
).toMatchInlineSnapshot(`"<Story name="Default" children={someTemplate} />"`);
});

it("transforms 'template' prop to 'children' and text expression becomes expression tag with identifier to snippet (case with invalid identifier)", async ({
expect,
}) => {
const code = `
<script context="module">
import { Story } from "@storybook/addon-svelte-csf";
</script>
<Story name="Default" template="some template with non valid idenitifier" />
`;
const component = await parseAndExtractSvelteNode<SvelteAST.Component>(code, 'Component');

expect(
print(
transformLegacyStory({
component,
state: { componentIdentifierName: {} },
})
)
).toMatchInlineSnapshot(`"<Story name="Default" children={template_c0gseq} />"`);
});

it("when directive 'let:args' is used then it wraps Story fragment with 'children' snippet block", async ({
expect,
}) => {
Expand Down
5 changes: 2 additions & 3 deletions src/compiler/pre-transform/codemods/legacy-story.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { camelCase } from 'es-toolkit/string';

import {
createASTArrayExpression,
createASTAttribute,
Expand All @@ -11,6 +9,7 @@ import {
} from '$lib/parser/ast.js';
import { InvalidTemplateAttribute } from '$lib/utils/error/legacy-api/index.js';

import { hashTemplateName } from '$lib/utils/identifier-utils';
import type { State } from '..';

interface Params {
Expand Down Expand Up @@ -253,7 +252,7 @@ function templateToChildren(
value: [
createASTExpressionTag({
type: 'Identifier',
name: camelCase(
name: hashTemplateName(
value[0].type === 'Text'
? value[0].data
: ((value[0].expression as ESTreeAST.Literal).value as string)
Expand Down
21 changes: 21 additions & 0 deletions src/compiler/pre-transform/codemods/template-to-snippet.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,27 @@ describe(transformTemplateToSnippet.name, () => {
`);
});

it("covers a case with provided prop 'id' and prop `id` not being a valid identifier", async ({
expect,
}) => {
const code = `
<script context="module" lang="ts">
import { Template } from "${pkg.name}";
</script>
<Template id="cool-template" let:args>
<Button {...args} variant="primary" />
</Template>
`;
const component = await parseAndExtractSvelteNode<SvelteAST.Component>(code, 'Component');

expect(print(transformTemplateToSnippet({ component }))).toMatchInlineSnapshot(`
"{#snippet template_haitqt(args)}
<Button {...args} variant="primary" />
{/snippet}"
`);
});

it("works with 'let:context' directive", async ({ expect }) => {
const code = `
<script context="module" lang="ts">
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/pre-transform/codemods/template-to-snippet.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { getStringValueFromAttribute } from '$lib/parser/analyse/story/attributes.js';
import type { SvelteAST } from '$lib/parser/ast.js';
import { hashTemplateName } from '$lib/utils/identifier-utils';

interface Params {
component: SvelteAST.Component;
Expand Down Expand Up @@ -70,7 +71,7 @@ export function transformTemplateToSnippet(params: Params): SvelteAST.SnippetBlo
type: 'SnippetBlock',
expression: {
type: 'Identifier',
name: id ?? 'sb_default_template',
name: id ? hashTemplateName(id) : 'sb_default_template',
},
parameters,
body: fragment,
Expand Down
20 changes: 20 additions & 0 deletions src/utils/identifier-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,23 @@ export const isValidVariableName = (str: string) => {

return true;
};

/**
* Function to convert a non valid string template name to a valid identifier preventing
* clashing with other templates with similar names.
*
* Stolen with 🧡 from the svelte codebase by @paoloricciuti
*
* @param str the template name
* @returns a hash based on the content of the initial string which is a valid identifier
*/
export function hashTemplateName(str: string) {
if (isValidVariableName(str)) return str;

str = str.replace(/\r/g, '');
let hash = 5381;
let i = str.length;

while (i--) hash = ((hash << 5) - hash) ^ str.charCodeAt(i);
return `template_${(hash >>> 0).toString(36)}`;
}

0 comments on commit f96d9f9

Please sign in to comment.