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

svelte-package removes type="application/ld+json" from script tags #6862

Closed
TeemuKoivisto opened this issue Sep 17, 2022 · 4 comments · Fixed by #6887
Closed

svelte-package removes type="application/ld+json" from script tags #6862

TeemuKoivisto opened this issue Sep 17, 2022 · 4 comments · Fixed by #6887
Labels
bug Something isn't working pkg:svelte-package Issues related to svelte-package

Comments

@TeemuKoivisto
Copy link

Describe the bug

Running svelte-package removes type="application/ld+json" from script tags even with preserve: ['ld+json'] using:

<svelte:head>
  {@html `<script type="application/ld+json" >${JSON.stringify(jsonLd)}</script>`}
</svelte:head>

is compiled to:

<svelte:head>
  {@html `<script >${JSON.stringify(jsonLd)}</script>`}
</svelte:head>

Reproduction

https://github.com/TeemuKoivisto/svelte-package-removes-jsonld-type

Logs

No response

System Info

System:
    OS: macOS 12.5.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 75.28 MB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.17.0 - ~/.nvm/versions/node/v16.17.0/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v16.17.0/bin/yarn
    npm: 8.15.0 - ~/.nvm/versions/node/v16.17.0/bin/npm
  Browsers:
    Chrome: 105.0.5195.125
    Firefox: 104.0.1
    Safari: 15.6.1
  npmPackages:
    @sveltejs/kit: ^1.0.0-next.484 => 1.0.0-next.484 
    @sveltejs/package: ^1.0.0-next.3 => 1.0.0-next.3 
    svelte: ^3.50.1 => 3.50.1 
    vite: ^3.1.1 => 3.1.1

Severity

serious, but I can work around it

Additional Information

No response

@benmccann benmccann added bug Something isn't working pkg:svelte-package Issues related to svelte-package labels Sep 17, 2022
@dominikg
Copy link
Member

The cause of this is that parsing script blocks to pass to preprocessors is done in a very fast and simplistic way. It also finds script tags nested in strings.

The workaround you linked to breaks apart the script tags in the sourcecode, so that they are not found by the parser, not passed through svelte-preprocess - which ultimately strips the type.

You already found the preserve option, but i think it would have to be preserve: 'application/ld+json' unfortunately it looks like that produces a different error

> Cannot find module './transformers/ld+json'
Require stack:
- /home/dominikg/develop/reproductions/svelte-package-removes-jsonld-type/node_modules/.pnpm/svelte-preprocess@4.10.7_52dhhvjbzvlifk77xwmrmntvea/node_modules/svelte-preprocess/dist/autoProcess.js
- /home/dominikg/develop/reproductions/svelte-package-removes-jsonld-type/node_modules/.pnpm/svelte-preprocess@4.10.7_52dhhvjbzvlifk77xwmrmntvea/node_modules/svelte-preprocess/dist/index.js
Require stack:
- /home/dominikg/develop/reproductions/svelte-package-removes-jsonld-type/node_modules/.pnpm/svelte-preprocess@4.10.7_52dhhvjbzvlifk77xwmrmntvea/node_modules/svelte-preprocess/dist/autoProcess.js
- /home/dominikg/develop/reproductions/svelte-package-removes-jsonld-type/node_modules/.pnpm/svelte-preprocess@4.10.7_52dhhvjbzvlifk77xwmrmntvea/node_modules/svelte-preprocess/dist/index.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:956:15)

so for now if you have to inject script tags via svelte:head + @html - which i am not too fond of in general. The workaround with splitting the script tag is what makes it work.

In a future version of svelte and/or svelte-preprocess, the way script tags are processed might change to avoid this.

There isn't much sveltekit or svelte-package could do about this, the issue should be transferred to either svelte or svelte-preprocess repo, or maybe closed with a ref to an existing issue related to this
eg sveltejs/svelte-preprocess#305 or sveltejs/svelte#5292

@TeemuKoivisto
Copy link
Author

@dominikg okay, yeah I think passing preserve should preserve the attributes as well so svelte-preprocess might be the right place. Agree about injecting script tags not being ideal yet that's how they decided to distribute the graph data.

dummdidumm added a commit that referenced this issue Sep 19, 2022
@dummdidumm
Copy link
Member

This is a bug in svelte-package, adding preserve: ['ld+json'] should work.

Rich-Harris pushed a commit that referenced this issue Sep 19, 2022
* [fix] don't strip type="application/.." tags

Fixes #6862

* changeset
@TeemuKoivisto
Copy link
Author

Yup, fixed with the latest package, amazing - thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pkg:svelte-package Issues related to svelte-package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants