sveltejs / prettier-plugin-svelte

Format your svelte components using prettier.
MIT License
717 stars 96 forks source link

Use an actual HTML parser: unexpected behavior around nested `<style>` / `script` tags #70

Open Prinzhorn opened 4 years ago

Prinzhorn commented 4 years ago

Obligatory https://stackoverflow.com/a/1732454/1422124

Took me a while to track this down, because prettier was also swallowing the error message (I'll investigate and open an issue in the appropriate repo).

Essentially the offending code I had was something like this (adapted from https://github.com/rmariuzzo/react-new-window/blob/2417b08352f34f193cc5e69524379fb70ccecf5b/src/NewWindow.js#L195 for svelte):

<script>
  Array.from(document.styleSheets).forEach(styleSheet => {
    // For <style> elements
    let rules;
    try {
      rules = styleSheet.cssRules;
    } catch (err) {
      console.error(err);
    }
    if (rules) {
      //...
    } else if (styleSheet.href) {
      // for <link> elements loading CSS from a URL
      ...
    }
  });
</script>

<style>...</style>

<div>...</div>

The comment For <style> elements inside the JavaScript causes the regex based parser to think it's a style tag.

Sure it's an edge case, but it might be worth considering switching to an actual HTML parser. Maybe svelte core can be imported into this plugin? I'm sure they're handling this already.

Thanks for the plugin by the way!

Prinzhorn commented 4 years ago

I'll submit a PR to upgrade the regex as a workaround. An actual parser might be overkill anyway.

Edit: This is a damn rabbit hole (shockingly). I thought we could adjust the regex to only match tags at the start of a line, but that would still not allow closing tags anywhere inside the content. So Chomsky won once again, I am defeated. I'm interested in what you are suggesting and if there is an easy way to integrate svelte's own parsing in here.

Anyway, the main issue I guess is that I didn't get any meaningful error message at all. Otherwise I'd just removed the comment ¯_(ツ)_/¯

dummdidumm commented 3 years ago

Also causes this behavior:

Input:

{@html `<style class="block">
...
</style>`}

Output:

{@html `<style class="block ✂prettier:content✂="LmFuaW1hdGlvbk5hbWUgewogI..."></style>`}

Workaround: Hide it in a nested string.

{@html `<${'style'} class="block">
...
</style>`}

(from https://github.com/sveltejs/prettier-plugin-svelte/issues/59#issuecomment-643336817)

codingwithchris commented 2 years ago

@dummdidumm @Prinzhorn This issue still exists as of:

"prettier": "^2.6.2",
"prettier-plugin-svelte": "^2.7.0",

Example:

Input

import { getCssText } from '@tokens';
<svelte:head>
    {@html `<style>${getCssText()}</style>`}
</svelte:head>

Output

import { getCssText } from '@tokens';
<svelte:head>
    {@html `<style ✂prettier:content✂="JHtnZXRDc3NUZXh0KCl9"></style>`}
</svelte:head>
secondfry commented 2 years ago

How come this be an enhancement tho, seems like a critical bug.

tadeohepperle commented 1 year ago

I also have this issue. Are there any news on this? How can I disable this behavior? It's potentially fucking up code pretty badly that could end up in production, just by accidentally saving with prettier.

justingolden21 commented 1 year ago

Can't add dynamic ld json schema with this bug that's been here for 3+ years

justingolden21 commented 1 year ago

Minimum reproducible

{@html `<style>${'a'}</style>`}

To confirm, this is caused by the vs code plugin for prettier.

dummdidumm commented 1 year ago

Do you have the latest version of the plugin/extension installed? Can't reproduce this given your snippet. If there's something that can be reproduced, please open a separate issue for that.

digvijayad commented 11 months ago

@dummdidumm For me, the components' top level script tag was converted to prettier:content Input:

<script>
    import ApplicationLogo from "@/Components/ApplicationLogo.svelte";
    import { inertia } from "@inertiajs/svelte";
    let className;
    export { className as class };
</script>

output:

<script
    ✂prettier:content✂="CiAgICBpbXBvcnQgQnJlZXplQXBwbGljYXRpb25Mb2dvIGZyb20gIkAvQ29tcG9uZW50cy9BcHBsaWNhdGlvbkxvZ28uc3ZlbHRlIjsKICAgIGltcG9ydCB7aW5lcnRpYX0gZnJvbSAiQGluZXJ0aWFqcy9zdmVsdGUiOwoKICAgIGxldCBjbGFzc05hbWU7CiAgICBleHBvcnQgeyBjbGFzc05hbWUgYXMgY2xhc3MgfTsK">{
}
</script>

Can reproduce using PhpStorm in 2 different scenarios:

  1. Reformat Whole File with prettier plugin installed using the Ctrl+Alt+Shift+L shortcut. Not via Ctrl+Shift+Alt+P To be specific only happens if Optimize Imports option is checked. image

  2. Using Git Commit Checks with either Reformat code or Optimize Imports options checked. image

I have disabled those options for myself, but hopefully that's something. I can put this into a new issue, but I already see 3 issues with similar content.

abdelfattahradwan commented 8 months ago

@dummdidumm For me, the components' top level script tag was converted to prettier:content Input:

<script>
    import ApplicationLogo from "@/Components/ApplicationLogo.svelte";
    import { inertia } from "@inertiajs/svelte";
    let className;
    export { className as class };
</script>

output:

<script
    ✂prettier:content✂="CiAgICBpbXBvcnQgQnJlZXplQXBwbGljYXRpb25Mb2dvIGZyb20gIkAvQ29tcG9uZW50cy9BcHBsaWNhdGlvbkxvZ28uc3ZlbHRlIjsKICAgIGltcG9ydCB7aW5lcnRpYX0gZnJvbSAiQGluZXJ0aWFqcy9zdmVsdGUiOwoKICAgIGxldCBjbGFzc05hbWU7CiAgICBleHBvcnQgeyBjbGFzc05hbWUgYXMgY2xhc3MgfTsK">{
}
</script>

Can reproduce using PhpStorm in 2 different scenarios:

  1. Reformat Whole File with prettier plugin installed using the Ctrl+Alt+Shift+L shortcut. Not via Ctrl+Shift+Alt+P To be specific only happens if Optimize Imports option is checked. image
  2. Using Git Commit Checks with either Reformat code or Optimize Imports options checked. image

I have disabled those options for myself, but hopefully that's something. I can put this into a new issue, but I already see 3 issues with similar content.

Oh, THANK YOU! This has been bugging me for weeks!

NeilOnasch0402 commented 2 months ago

Should be treated as a bug, not an enhancement