fix(svelte): Track components without <script> tags #5957
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| function hasScriptTag(content: string): boolean { | ||
| // This regex is taken from the Svelte compiler code. | ||
| // They use this regex to find matching script tags that are passed to the `script` preprocessor hook: | ||
| // https://github.com/sveltejs/svelte/blob/bb83eddfc623437528f24e9fe210885b446e72fa/src/compiler/preprocess/index.ts#L144 |
There was a problem hiding this comment.
Just out of curiosity: Do you know why they have multiline comments like that in their regex?
There was a problem hiding this comment.
I was also wondering about that. Traced it back yesterday to a commit saying that this helps them actually getting rid of comments later on. I didn't reach a point where I was entirely convinced but i suspect that it has something to do with how they process the "replacements" they identify, leading them to do nothing if there is nothing to preprocess here
| // However, we remove the first part of the regex to not match HTML comments | ||
| const scriptTagRegex = /<script(\s[^]*?)?(?:>([^]*?)<\/script>|\/>)/gi; | ||
|
|
||
| // Regex testing is not a super safe way of checking for the presence of a <script> tag in the Svelte |
There was a problem hiding this comment.
I agree this is a bit sketchy. Non blocking but have you looked at import { parse } from 'svelte/compiler'; if it outputs something we could use that is a little bit more robust?
There was a problem hiding this comment.
I was just thinking about that too on my way to the train (apparently, walking really helps some time 😅). Will take a look and see if anything comes out of it
There was a problem hiding this comment.
Seems much more elegant and safer to use svelte's parser. Changed it in 31f1c52.
size-limit report 📦
|
Lms24
force-pushed
the
lms-fix-svelte-no-script-components-ii
branch
from
31f1c52 to
ed12846
Compare
| // They use this regex to find matching script tags that are passed to the `script` preprocessor hook: | ||
| // https://github.com/sveltejs/svelte/blob/bb83eddfc623437528f24e9fe210885b446e72fa/src/compiler/preprocess/index.ts#L144 | ||
| // However, we remove the first part of the regex to not match HTML comments | ||
| const scriptTagRegex = /<script(\s[^]*?)?(?:>([^]*?)<\/script\s*>|\/>)/gi; |
Check failure
Code scanning / CodeQL
Bad HTML filtering regexp
|
For future readers: Using Svelte's parse functionality didn't work. The problem was that Svelte's parser only parses JS but not typescript, meaning that scripts written in TS ( This is by design, as Svelte provides an official preprocessor to transpile TS to JS before the .svelte file is parsed. Similarly to our 2-pass approach though, we can't rely on this preprocessor because for TS->JS transpilation, it uses the Therefore, we'll revert back to the regex approach. Last solution: Use a 3rd party library for parsing the entire *.svelte file as HTML and check if a valid top-level <script> tag exists. We'll explore this, if the regex approach causes problems. |
This PR makes our Svelte component tracking feature track components without
<script>tags. Previously these components would not be picked up by our preprocessor because itsscripthook is only called for components with script blocks. With this PR, we first leverage themarkuphook to check if a script block exists. If it doesn't, we add an empty script tag which we later revisit in thescript hookwhere we inject the component tracking code.Note: To determine if a script tag exists in the file, we use a regex check which has drawbacks, as it is not 100% bullet proof. However, we decided that for the lack of better options as explained in #5923(comment) we'll go with this approach for now. Also, the svelte compiler internally uses the same regex to detect
<script>tags.This PR adds additional tests to check the new behaviour (brings up preprocessor coverage to 100%)
ref: #5923