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

Draft: Backwards compatible transport types #156

Closed

Conversation

liamjones
Copy link
Contributor

I've started looking at #142.

Question; how far back do we want backward compatibility in the types?

Currently, looking at the code comments in createSentryTransport it looks like we're trying to support all the way back to v4? Or are we only interested in v6 and v7 nowadays? Asking because more backwards compatibility = more work. :)

Progress so far;

I've added some type tests for the transport, these will currently pass due to the any return type on createSentryTransport. If you were to remove the any return then you'd see the new test file passing for v7 but failing for 6, 5 and 4.

I've had to add these tests as a separate build-test script with it's own TypeScript project because the tests can't type-check without a build producing dist/** (due to the CJS and ESM tests using dist) and doing a build runs the tests; a catch-22.

@liamjones liamjones marked this pull request as draft January 1, 2023 11:00
@liamjones
Copy link
Contributor Author

@zivl Any thoughts on my question?

(I'm also debating if I should get these tests merged anyway in advance of looking at any type improvements in a separate PR...)

@zivl
Copy link
Owner

zivl commented Jan 10, 2023

hey @liamjones
I guess we would like to support sentry v7 & v6, no need for earlier versions.
I also looked at your tests - looks pretty neat :)

I really appreciate your help on this!! let me know when you're ready for final review!

@github-actions
Copy link
Contributor

Stale pull request message

@github-actions
Copy link
Contributor

Stale pull request message

@zivl
Copy link
Owner

zivl commented May 21, 2023

@liamjones hey
this PR was (automatically) closed due to inactivity. Are you still working on it? do you want me to re-open this?

@liamjones
Copy link
Contributor Author

HI @zivl,

Sorry, I've not found time to look at it recently. Feel free to let it autoclose if you want, I can always re-raise it in future.

@github-actions
Copy link
Contributor

Stale pull request message

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

Successfully merging this pull request may close these issues.

None yet

2 participants