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

feat(typescript): Upgrade to TS v4 (back compatible with TS v3.8) #2995

Merged
merged 27 commits into from
Apr 25, 2023

Conversation

krystofwoldrich
Copy link
Member

📢 Type of change

  • Enhancement

📜 Description

The library compiles using TS v4 and from v4 declaration files, v3.8 files are created using downlevel-dts.
Library ships with both the new and old types.
CI checks if the downleveled types are working correctly with 3.8 tsc.

💡 Motivation and Context

closes: #2983

💚 How did you test it?

ci, test project

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • All tests passing
  • No breaking changes

🔮 Next steps

@github-actions
Copy link
Contributor

github-actions bot commented Apr 21, 2023

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1250.82 ms 1259.54 ms 8.72 ms
Size 2.36 MiB 2.84 MiB 489.63 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
dadc233+dirty 1223.20 ms 1236.88 ms 13.68 ms
ad6c299+dirty 1244.76 ms 1260.10 ms 15.34 ms
d197b5c+dirty 1217.61 ms 1242.66 ms 25.05 ms
9a3ca65+dirty 1247.06 ms 1274.58 ms 27.52 ms
d7401ac+dirty 1252.38 ms 1275.04 ms 22.66 ms
15c80ab+dirty 1223.74 ms 1228.96 ms 5.22 ms
52a8031+dirty 1280.88 ms 1289.78 ms 8.90 ms
80b2ce3+dirty 1265.92 ms 1268.60 ms 2.69 ms
e73f4ed+dirty 1243.27 ms 1244.52 ms 1.25 ms
0db0c72+dirty 1275.02 ms 1285.84 ms 10.82 ms

App size

Revision Plain With Sentry Diff
dadc233+dirty 2.36 MiB 2.84 MiB 486.85 KiB
ad6c299+dirty 2.36 MiB 2.84 MiB 488.85 KiB
d197b5c+dirty 2.36 MiB 2.82 MiB 462.86 KiB
9a3ca65+dirty 2.36 MiB 2.82 MiB 462.89 KiB
d7401ac+dirty 2.36 MiB 2.83 MiB 481.14 KiB
15c80ab+dirty 2.36 MiB 2.83 MiB 474.49 KiB
52a8031+dirty 2.36 MiB 2.82 MiB 469.44 KiB
80b2ce3+dirty 2.36 MiB 2.84 MiB 486.98 KiB
e73f4ed+dirty 2.36 MiB 2.82 MiB 469.44 KiB
0db0c72+dirty 2.36 MiB 2.84 MiB 487.01 KiB

Comment on lines 68 to 75
//this is 4.0 feature and cant be downleveled
type Strings = [string, string];
type Numbers = [number, number];
export type StrStrNumNumBool = [...Strings, ...Numbers, boolean];

//this will be downleveled
type World = "world";
export type Greeting = `hello ${World}`;
Copy link
Member Author

@krystofwoldrich krystofwoldrich Apr 24, 2023

Choose a reason for hiding this comment

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

This is an example of code that will be downleveled and code that can not be downleveled. When CI is done, I will link its output.

Check job: https://github.com/getsentry/sentry-react-native/actions/runs/4789050823/jobs/8516494872

Error on line 71

Screenshot 2023-04-24 at 19 07 58

type from 75 was downleveled:

declare type World = "world";
export declare type Greeting = string;
export {};
//# sourceMappingURL=sdkinfo.d.ts.map

@krystofwoldrich krystofwoldrich marked this pull request as ready for review April 24, 2023 17:17
@github-actions
Copy link
Contributor

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 326.17 ms 354.28 ms 28.11 ms
Size 17.73 MiB 19.75 MiB 2.02 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
dadc233+dirty 333.78 ms 343.94 ms 10.16 ms
d197b5c+dirty 338.94 ms 354.87 ms 15.93 ms
9a3ca65+dirty 326.93 ms 330.14 ms 3.21 ms
d7401ac+dirty 375.20 ms 383.51 ms 8.31 ms
80b2ce3 385.02 ms 387.36 ms 2.34 ms
15c80ab+dirty 336.27 ms 350.58 ms 14.31 ms
b1e8712 462.11 ms 465.71 ms 3.60 ms
52a8031+dirty 311.55 ms 321.37 ms 9.82 ms
ad6c299 375.94 ms 382.02 ms 6.08 ms
e73f4ed+dirty 332.96 ms 354.33 ms 21.37 ms

App size

Revision Plain With Sentry Diff
dadc233+dirty 17.73 MiB 19.75 MiB 2.02 MiB
d197b5c+dirty 17.73 MiB 20.04 MiB 2.31 MiB
9a3ca65+dirty 17.73 MiB 20.04 MiB 2.31 MiB
d7401ac+dirty 17.73 MiB 19.75 MiB 2.02 MiB
80b2ce3 17.73 MiB 19.75 MiB 2.02 MiB
15c80ab+dirty 17.73 MiB 20.04 MiB 2.31 MiB
b1e8712 17.73 MiB 19.75 MiB 2.02 MiB
52a8031+dirty 17.73 MiB 20.04 MiB 2.31 MiB
ad6c299 17.73 MiB 19.75 MiB 2.02 MiB
e73f4ed+dirty 17.73 MiB 20.04 MiB 2.31 MiB

Copy link
Contributor

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

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

E2E test is unhappy, otherwise LGTM.

@krystofwoldrich
Copy link
Member Author

@marandaneto Thanks check it, seems just like flakiness, yarn install failed. I'll re-run it.

Co-authored-by: Manoel Aranda Neto <[email protected]>
@krystofwoldrich krystofwoldrich changed the title Upgrade to Typescript v4 feat(typescript): Upgrade to TS v4 (back compatible with TS v3.8) Apr 25, 2023
@krystofwoldrich krystofwoldrich enabled auto-merge (squash) April 25, 2023 09:56
@github-actions
Copy link
Contributor

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 490c50b

@krystofwoldrich krystofwoldrich merged commit 3ffcddd into main Apr 25, 2023
@krystofwoldrich krystofwoldrich deleted the kw-upgrade-ts-4 branch April 25, 2023 10:06
@github-actions
Copy link
Contributor

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 335.50 ms 365.82 ms 30.32 ms
Size 7.15 MiB 8.04 MiB 912.17 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
dadc233+dirty 363.19 ms 370.37 ms 7.18 ms
ad6c299+dirty 336.47 ms 362.89 ms 26.42 ms
d197b5c+dirty 258.75 ms 313.61 ms 54.86 ms
9a3ca65+dirty 344.96 ms 358.92 ms 13.96 ms
d7401ac+dirty 373.98 ms 394.08 ms 20.10 ms
15c80ab+dirty 276.38 ms 327.54 ms 51.17 ms
52a8031+dirty 330.72 ms 358.76 ms 28.03 ms
80b2ce3+dirty 271.29 ms 316.47 ms 45.18 ms
e73f4ed+dirty 262.98 ms 311.02 ms 48.04 ms
0db0c72+dirty 335.20 ms 351.06 ms 15.86 ms

App size

Revision Plain With Sentry Diff
dadc233+dirty 7.15 MiB 8.04 MiB 910.84 KiB
ad6c299+dirty 7.15 MiB 8.04 MiB 912.17 KiB
d197b5c+dirty 7.15 MiB 8.09 MiB 962.72 KiB
9a3ca65+dirty 7.15 MiB 8.09 MiB 962.83 KiB
d7401ac+dirty 7.15 MiB 8.04 MiB 910.85 KiB
15c80ab+dirty 7.15 MiB 8.09 MiB 966.13 KiB
52a8031+dirty 7.15 MiB 8.09 MiB 965.95 KiB
80b2ce3+dirty 7.15 MiB 8.04 MiB 911.02 KiB
e73f4ed+dirty 7.15 MiB 8.09 MiB 965.94 KiB
0db0c72+dirty 7.15 MiB 8.04 MiB 911.02 KiB

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.

Upgrade TS to v4
2 participants