Skip to content
This repository was archived by the owner on Aug 30, 2023. It is now read-only.

chore(deps): Downgrade typescript & jest to versions used in sentry-javascript#310

Merged
mydea merged 15 commits intomainfrom
fn/downgrade-ts
Nov 21, 2022
Merged

chore(deps): Downgrade typescript & jest to versions used in sentry-javascript#310
mydea merged 15 commits intomainfrom
fn/downgrade-ts

Conversation

@mydea
Copy link
Copy Markdown
Member

@mydea mydea commented Nov 16, 2022

For reasons™️ we are stuck with an old version of typescript (3.8) in sentry-javascript.
In order to prepare the migration there, this PR downgrades typescript to that version as well.
With it, that required some other changes, mainly also downgrading jest, which required adapting some tests... 😢 not ideal, but I tried to make it work with as little changes as possible.

The two biggest issues were:

  • jest 27 does not support jest.useFakeTimers({ autoAdvance: true }), so needed to work around that somehow. I tried to replicate this manually
  • jest 27 does not support expect.closeTo(). This was used once, I commented this out for now.

Other things:

  • @ts-expect-error is not supported, so instead we use @ts-ignore
  • eslint also needed downgrading
  • I had issues with @jest/globals (which we also don't use in sentry-javascript), so I changed this to use the globals everywhere

@mydea mydea added the dependencies Pull requests that update a dependency file label Nov 16, 2022
@mydea mydea requested review from Lms24 and billyvg November 16, 2022 15:34
@mydea mydea self-assigned this Nov 16, 2022
@mydea
Copy link
Copy Markdown
Member Author

mydea commented Nov 16, 2022

@billyvg maybe you could take a look at the failing test? I am lacking a bit of context on that test (e.g. what exactly the timing of stuff should be etc), so having a hard time debugging this 😅 also the "replacement" for the autoAdvance stuff is def. a bit "hacky", so maybe needs adjustment for specific tests I guess...

@mydea
Copy link
Copy Markdown
Member Author

mydea commented Nov 16, 2022

I think the size failed CI job can be ignored, that's because master has a different ts version leading to issues.

@billyvg
Copy link
Copy Markdown
Member

billyvg commented Nov 16, 2022

Weird, tests are passing locally for me

@mydea mydea force-pushed the fn/downgrade-ts branch 2 times, most recently from f875718 to e0eb773 Compare November 17, 2022 09:45
@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 17, 2022

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
dist/index.js 34.88 KB (+1.59% 🔺) 698 ms (+1.59% 🔺) 70 ms (-30.06% 🔽) 767 ms

@mydea
Copy link
Copy Markdown
Member Author

mydea commented Nov 17, 2022

Tests seem to be passing now! Had to do some further adjustments for the build as well, but seems all good now.

Copy link
Copy Markdown
Member

@billyvg billyvg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me -- can we separate out removing @jest/globals into its own PR? I would like to have it in its own commit so it's easy to reference/revert down the road.

@mydea
Copy link
Copy Markdown
Member Author

mydea commented Nov 18, 2022

This looks good to me -- can we separate out removing @jest/globals into its own PR? I would like to have it in its own commit so it's easy to reference/revert down the road.

👍 will do! see: #319

mydea added a commit that referenced this pull request Nov 18, 2022
We do not use it in sentry-javascript (although it is actually nice! but
for ease of transition), so replacing this here.

Extracted from: #310
@mydea mydea merged commit 682b06d into main Nov 21, 2022
@mydea mydea deleted the fn/downgrade-ts branch November 21, 2022 08:02
mydea added a commit to getsentry/sentry-javascript that referenced this pull request Nov 23, 2022
We do not use it in sentry-javascript (although it is actually nice! but
for ease of transition), so replacing this here.

Extracted from: getsentry/sentry-replay#310
mydea added a commit to getsentry/sentry-javascript that referenced this pull request Nov 23, 2022
…avascript (getsentry/sentry-replay#310)

For reasons™️ we are stuck with an old version of typescript (3.8) in
sentry-javascript.
In order to prepare the migration there, this PR downgrades typescript
to that version as well.
With it, that required some other changes, mainly also downgrading jest,
which required adapting some tests... 😢 not ideal, but I tried to make
it work with as little changes as possible.

The two biggest issues were:

* jest 27 does not support `jest.useFakeTimers({ autoAdvance: true })`,
so needed to work around that somehow. I tried to replicate this
manually
* jest 27 does not support `expect.closeTo()`. This was used once, I
commented this out for now.

Other things:

* `@ts-expect-error` is not supported, so instead we use `@ts-ignore`
* eslint also needed downgrading
* I had issues with `@jest/globals` (which we also don't use in
sentry-javascript), so I changed this to use the globals everywhere

Co-authored-by: Billy Vong <billyvg@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants