Skip to content

Conversation

@florian-lefebvre
Copy link

Closes #10309

  • If you've added code that should be tested, please add tests.
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

I have not added tests just yet because I don't know how to mock virtual imports. I'm waiting for an answer from the Astro Discord.

@florian-lefebvre
Copy link
Author

Do you have something like changesets? Couldn't find it

@AbhiPrasad
Copy link
Member

@florian-lefebvre we handwrite our changelogs since we rely on gitflow + our release tool craft - feel free to open up the PR we'll take care of the changelog!

@Lms24
Copy link
Member

Lms24 commented Jan 25, 2024

Hey @florian-lefebvre thanks for opening this PR! I only skimmed it as I don't have much time at the moment but will take a closer look hopefully tomorrow. Nice idea with the virtualized config module!

I ran CI and looks like there are some build issues in the Astro package. Are you able to reproduce this locally?

@florian-lefebvre
Copy link
Author

A few reasons, but idk how to fix them:

  1. [tunnel.ts] Globals are not recognized: fetch, URL, Response, Request
  2. [virtual.d.ts] Looks like the type import does not work
  3. [virtual-imports.ts] Not sure how to fix eslint errors

Regarding testing, there's something about virtual modules (see docs). I've almost never tested Astro/integration code before so I'm not sure what's the best approach here

@Lms24
Copy link
Member

Lms24 commented Dec 12, 2024

I'll close this PR as part of a larger issue and PR cleanup throughout the Sentry repo. Sorry for not getting back to you back then, unfortunately, priorities didn't align well. I think this is a nice feature but it would need some more refinement to ship this. Let me know if you want to give this another shot. Happy to reopen :)

@Lms24 Lms24 closed this Dec 12, 2024
@florian-lefebvre florian-lefebvre deleted the feat/astro-tunnel branch December 12, 2024 19:13
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

Successfully merging this pull request may close these issues.

Add tunneling option to @sentry/astro

3 participants