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

Conversation

paoloricciuti
Copy link
Contributor

This fix the transform to handle the case where the template name is an invalid identifier.

Before it was using camelCase (only for the Story actually so that was also a bug) but the problem with camelCase is that it can lead to the same name from different string. For example Modal && Trigger and Modal || Trigger have the same camelCase.

To fix this i used a small hash function (stolen from the svelte code base) so that if the template is not a valid identifier (which needs to be to be a Snippet) it convert it to something like template_skajsh (which we don't care too much except for stack traces).

I don't know if it's a solution that you like but it works.

@JReinhold JReinhold self-assigned this Oct 31, 2024
@JReinhold JReinhold added bug Something isn't working patch Increment the patch version when merged labels Oct 31, 2024
@xeho91
Copy link
Collaborator

xeho91 commented Nov 1, 2024

🧡

Apologies for missing the case where the transformation of the legacy template component id needs to be a valid identifier for snippet. I know you mentioned it on Discord!


BTW, Jeppe. We could implement some formatting CI to this repo (Prettier probably is the best choice since it supports Svelte, unlike Biome). I'm saying this because I've noticed formatting changes in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working patch Increment the patch version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants