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

fix: properly transform invalid identifiers #226

Open
wants to merge 1 commit into
base: next
Choose a base branch
from
Open
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
26 changes: 23 additions & 3 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 Expand Up @@ -280,9 +302,7 @@ describe(transformLegacyStory.name, () => {
`);
});

it("leaves existing Story parameters untouched", async ({
expect,
}) => {
it('leaves existing Story parameters untouched', async ({ expect }) => {
const code = `
<script context="module">
import { Story } from "@storybook/addon-svelte-csf";
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 '#parser/ast';
import { InvalidTemplateAttribute } from '#utils/error/legacy-api/index';

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

interface Params {
Expand Down Expand Up @@ -247,7 +246,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
39 changes: 24 additions & 15 deletions src/compiler/pre-transform/codemods/template-to-snippet.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,7 @@ describe(transformTemplateToSnippet.name, () => {
`;
const component = await parseAndExtractSvelteNode<SvelteAST.Component>(code, 'Component');

expect(
print(
transformTemplateToSnippet({ component })
)
).toMatchInlineSnapshot(`
expect(print(transformTemplateToSnippet({ component }))).toMatchInlineSnapshot(`
"{#snippet sb_default_template(args)}
<Button {...args} variant="primary" />
{/snippet}"
Expand All @@ -43,17 +39,34 @@ describe(transformTemplateToSnippet.name, () => {
`;
const component = await parseAndExtractSvelteNode<SvelteAST.Component>(code, 'Component');

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

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 All @@ -66,11 +79,7 @@ describe(transformTemplateToSnippet.name, () => {
`;
const component = await parseAndExtractSvelteNode<SvelteAST.Component>(code, 'Component');

expect(
print(
transformTemplateToSnippet({ component })
)
).toMatchInlineSnapshot(`
expect(print(transformTemplateToSnippet({ component }))).toMatchInlineSnapshot(`
"{#snippet sb_default_template(_args, context)}
<p>{context.args}</p>
{/snippet}"
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 '#parser/analyse/story/attributes';
import type { SvelteAST } from '#parser/ast';
import { hashTemplateName } from '#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)}`;
}