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

Using sanitize reset in SvelteKit triggers error #3251

Closed
4 tasks done
TGlide opened this issue Oct 19, 2023 · 4 comments · Fixed by #3253
Closed
4 tasks done

Using sanitize reset in SvelteKit triggers error #3251

TGlide opened this issue Oct 19, 2023 · 4 comments · Fixed by #3253

Comments

@TGlide
Copy link
Contributor

TGlide commented Oct 19, 2023

UnoCSS version

0.56.5

Describe the bug

When using UnoCSS with SvelteKit, with the @unocss/svelte-scoped package, if you pass in @unocss/reset/sanitize/sanitize.css into the injectReset option, the following error appears. Switching to @unocss/reset/normalize.css triggers no errors.

WindowsTerminal_Y8fjCYMnUO

Reproduction

I assume no reproduction is needed from my description above. If one is needed, let me know and I'll happily provide one.

System Info

No response

Validations

@henrikvilhelmberglund
Copy link
Contributor

:where(nav li)::before {
  content: "\200B";
  float: left;
}

This thing breaks it, I have no idea why but you can delete it in the sanitize.css until it's fixed.

@TGlide
Copy link
Contributor Author

TGlide commented Oct 20, 2023

:where(nav li)::before {
  content: "\200B";
  float: left;
}

This thing breaks it, I have no idea why but you can delete it in the sanitize.css until it's fixed.

Thanks for the workaround!

@henrikvilhelmberglund
Copy link
Contributor

No problem!

It seems hard to fix in the reset package itself though because the files are not in this repo, instead they are brought in from the sanitize repo which seems unmaintained. It would probably have to be replaced in svelte-scoped when reading the .css reset files, replacing "\200B" with "​". aka a real zero width space character.

So... My boring suggestion is to just use another reset file for now. 😊
Perhaps @jacob-8 would like to fix this though since it's a bit confusing.

@jacob-8
Copy link
Contributor

jacob-8 commented Nov 6, 2023

Thanks for solving this @chu121su12!

I'm going to leave it alone for now because I don't understand the \200b very well yet, but I do want to write down my thoughts.

When it comes to those regexes, I would prefer to make them more readable like this:

export function replaceGlobalStylesPlaceholder(code: string, stylesTag: string) {
  // ...
  const escapedStylesTag = escapeBackslashesAndTemplateLiteralCharacters(stylesTag)
  return code.replace(QUOTES_WITH_PLACEHOLDER_RE, `\`${escapedStylesTag}\``)  
}

const BACKSLASH_NOT_FOLLOWED_BY_BACKTICK_OR_DOLLAR_SIGN = /\\(?![`$])/g
const UNESCAPED_BACKTICK_OR_DOLLAR_SIGN = /(?<!\\)([`$])/g

export function escapeBackslashesAndTemplateLiteralCharacters(code: string) {
  return code
    .replaceAll(BACKSLASH_NOT_FOLLOWED_BY_BACKTICK_OR_DOLLAR_SIGN, '\\\\')
    .replaceAll(UNESCAPED_BACKTICK_OR_DOLLAR_SIGN, '\\$1')
} 

I also would prefer just placing a global.test.ts file right beside with something like:

describe(escapeBackslashesAndTemplateLiteralCharacters, () => {
  it('to-fill-in', () => {
    const escapedUnicode = 'li::before { content: "\200B"; }'
    expect(escapeBackslashesAndTemplateLiteralCharacters(escapedUnicode)).toContain('\\200B')
  })
})

This is a small unit test that was placed the more high-level tests of the preprocessor over different use cases. I'm presuming you are using a .css file because Typescript won't actually run with a 'legacy octal escape sequence like \200B. But is that important to test this the root issue here? Is there another character sequence we could test instead?

Let me know if you can help me understand things better, otherwise I'm glad you had a fix for us here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants