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 Component Tracking doesn't pick up components without a <script> tag #5923

Closed
7 tasks done
Lms24 opened this issue Oct 10, 2022 · 3 comments · Fixed by #5936
Closed
7 tasks done

Svelte Component Tracking doesn't pick up components without a <script> tag #5923

Lms24 opened this issue Oct 10, 2022 · 3 comments · Fixed by #5936

Comments

@Lms24
Copy link
Member

Lms24 commented Oct 10, 2022

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which package are you using?

@sentry/svelte

SDK Version

7.15.0

Framework Version

Svelte 3

Link to Sentry event

No response

Steps to Reproduce

  1. Create Svelte Test App w/ a few components; some of them with <script> tags, some without

Expected Result

The SDK should record spans for all components that should be tracked, regardless of if they have a <script> tag or not.

Actual Result

The SDK only records spans for components with one (or multiple) <script> tag(s) (afaict, we handle the multiple script tags case fine).

My suspicion is that components w/o <script> are not sent through the script preprocessor hook we use. If that is the case, we should consider switching to another hook or (if impossible/infeasible) update docs to inform people that they need to include a script tag.

To Do:

After discussing this internally, we decided to make some (backwards-compatible) changes to how we handle sentry-specific svelte.config.js entries. We'll go with a more general, withSentryConfig approach that allows us to have more freedom with adding future Svelte preprocessors or other compile-time specific sentry items.

By taking this approach, we can also fix this specific bug in a more elegant and much safer way.

@Lms24 Lms24 changed the title Svelte Component Tracking doesn't pick up components without a <script> tag Svelte Component Tracking doesn't pick up components without a <script> tag Oct 10, 2022
@Lms24 Lms24 self-assigned this Oct 11, 2022
@Lms24
Copy link
Member Author

Lms24 commented Oct 12, 2022

Updated issue with To Do section

@Lms24
Copy link
Member Author

Lms24 commented Oct 13, 2022

Update: After actually implementing the 2nd pass preprocessor, I realized that the order in which preproc hooks are called is not as I thought. I thought that all hooks per preprocessor are executed, followed by the hooks of the next preproc:

basically:

p1.markup -> p1.script -> p2.markup -> p2.script

However, in reality, all markup hooks are executed before all script and style hooks:

p1.markup -> p2.markup -> p1.script -> p2.script

It's all in the Svelte docs btw, I should have just read this stuff more carefully :/

This means, our 2-pass approach won't work because for that, p1.script would need to be executed before p2.markup. So, we're back to the initial idea of injecting empty script tags in case there are none. This time, though, we'll rely on (almost) the same checking mechanism that svelte uses internally for finding <script> blocks.

Lms24 added a commit that referenced this issue Oct 14, 2022
Make our Svelte component tracking feature track components without `<script>` tags. Previously these components would not be picked up by our preprocessor because its `script` hook is only called for components with script blocks. With this PR, we first leverage the `markup` hook to check if a script block exists. If it doesn't, we add an empty script tag which we later revisit in the `script hook` where 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)](#5923 (comment)) we'll go with this approach for now. Also, the svelte compiler internally uses the same regex to detect `<script>` tags.

Add additional tests to check the new behaviour (brings up preprocessor coverage to 100%)
@Lms24
Copy link
Member Author

Lms24 commented Oct 18, 2022

This was fixed (#5957) and will be released with the next SDK release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment