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

Transformation error (Unexpected token) and wrong behaviours #52

Open
DarthGigi opened this issue May 16, 2024 · 3 comments
Open

Transformation error (Unexpected token) and wrong behaviours #52

DarthGigi opened this issue May 16, 2024 · 3 comments

Comments

@DarthGigi
Copy link

Environment

Svelte/Kit

Steps to Reproduce

  1. Run pnpx @sentry/migr8@latest
  2. Select Apply all transformations

Expected Result

The migrate command changes the files that are actually necessary to be changed and in the right way

Actual Result

For each component I have in my routes folder, I get the following error:

ERR /src/routes/bug-report.svelte Transformation error (Unexpected token (34:1))
SyntaxError: Unexpected token (34:1)

This is because for some reason </string> is being added just before </script> when transforming the files:
Screenshot 2024-05-16 at 9  43 31@2x

This doesn't happen in sveltekit's route files (+layout and +page.svelte) files, however, it does in +error.svelte files.

Another thing is that for some reason it tries to "fix" self-closing elements.
If we have <div /> for example, it changes it to <div></div>, while it is true that <div /> is not valid HTML, this should not apply to components:

Screenshot 2024-05-16 at 9  51 53@2x

Why is Sentry changing my markup? Isn't it supposed to just change it's own stuff and only in <script> tags?

Another thing you can see in the screenshot is that Sentry added </svelte:window> to the bottom of the file:

  1. <svelte:window/> does not accept children, so it must be always closed/self closed without anything as it's child
  2. <svelte:window/> is already closed, why does Sentry think it's not?

This migrate command changed about 170 files (mostly .svelte files since I'm using shadcn svelte components) which makes it not worth going over the stuff that Sentry changed to actually migrate from v7 to v8 and to check if there are any todo's left.

Feel free to change the title of this issue to a more fitting one.

@Lms24
Copy link
Member

Lms24 commented May 27, 2024

Hey @DarthGigi thanks for writing in and apologies for the late reply!

Why is Sentry changing my markup? Isn't it supposed to just change it's own stuff and only in <script> tags?

Definitely not on purpose. Ideally, we change as little as possible. My best guess is that our parser has problems with svelte files and I'm actually considering excluding svelte files from the transformations for the moment. At least in basic svelte apps there shouldn't be too much need to use Sentry code in svelte components.

 This is because for some reason is being added just before </script> when transforming the files

Hmm that's also a weird one. I'll check what's going wrong here.

@Florinstruct
Copy link

Florinstruct commented May 31, 2024

Same here. Migration script deletes or rewrites random seeming sections of our codebase. I tried to figure out which option ("transformer") is responsible, but more than one seem to trigger this.

Even if you only Add migration comments, the script will rewrite over 400 files and break 150 of them.

image

@DarthGigi
Copy link
Author

Any updates on this issue?

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

No branches or pull requests

3 participants