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

Bump TypeScript to 3.8.3 #1244

Merged
merged 1 commit into from
Mar 9, 2023
Merged

Conversation

mszabo-wikia
Copy link
Contributor

Which problem is this PR solving?

Short description of the changes

This is a conservative bump to minimize the amount of changes needed in tooling and dependencies. Version 3.8.3 was chosen because it is the version that implements support for type-only exports and imports (i.e. import type { TFoo } from '@types/foo';, which is the major source of incompatibility with upgraded type definitions in newer dependency versions.

Notable changes:

  • As noted in Upgrade React version - LFX Mentorship #998, the @types/react version used to type-check jaeger-ui is a transitive dependency on version 16.8.7, rather than the expected 18. Unfortunately, both newer 16.x type definitions as well as 18.x type definitions are incompatible with various dependencies such as antd. As a workaround, downgrade the typing versions for now and add an explicit dependency on them to the project. Only index.tsx was using a React 18-specific API (createRoot), so convert it back to JS until the typings can be updated again.
  • TypeScript now enforces that composite projects must also generate declaration files, since that's what the project references system uses. Make it so. This required a small adjustment to the ErrorMessage component, as the types of its Message and Details sub-components could not be reflected in this declaration file; this was trivially fixable by converting them to named exports instead (which is also consistent with other areas of the codebase).
  • Make Plexus a project reference in the root tsconfig, per the longstanding todo and per the changes above.

This is a conservative bump to minimize the amount of changes needed in
tooling and dependencies. Version 3.8.3 was chosen because it is the
version that implements support for type-only exports and imports (i.e.
`import type { TFoo } from '@types/foo';`, which is the major source of
incompatibility with upgraded type definitions in newer dependency
versions.

Notable changes:
* As noted in jaegertracing#998, the `@types/react` version used to type-check
  `jaeger-ui` is a transitive dependency on version `16.8.7`, rather
  than the expected `18`. Unfortunately, both newer `16.x` type
  definitions as well as `18.x` type definitions are incompatible
  with various dependencies such as `antd`. As a workaround, downgrade
  the typing versions for now and add an explicit dependency on them
  to the project. Only index.tsx was using a React 18-specific API
  (createRoot), so convert it back to JS until the typings can be
  updated again.
* TypeScript now enforces that `composite` projects must also generate
  declaration files, since that's what the project references system
  uses. Make it so. This required a small adjustment to the ErrorMessage
  component, as the types of its Message and Details sub-components
  could not be reflected in this declaration file; this was trivially
  fixable by converting them to named exports instead (which is also
  consistent with other areas of the codebase).
* Make Plexus a project reference in the root tsconfig, per the
  longstanding todo and per the changes above.

Signed-off-by: Máté Szabó <mszabo@fandom.com>
@codecov
Copy link

codecov bot commented Mar 8, 2023

Codecov Report

Patch coverage: 66.66% and project coverage change: +0.10 🎉

Comparison is base (6d281e9) 95.40% compared to head (cf926a7) 95.50%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1244      +/-   ##
==========================================
+ Coverage   95.40%   95.50%   +0.10%     
==========================================
  Files         244      244              
  Lines        7751     7749       -2     
  Branches     2017     2017              
==========================================
+ Hits         7395     7401       +6     
+ Misses        355      348       -7     
+ Partials        1        0       -1     
Impacted Files Coverage Δ
...src/components/TracePage/ArchiveNotifier/index.tsx 100.00% <ø> (ø)
packages/jaeger-ui/src/index.jsx 0.00% <0.00%> (ø)
...s/jaeger-ui/src/components/common/ErrorMessage.tsx 100.00% <100.00%> (ø)
...mponents/TracePage/TraceStatistics/tableValues.tsx 96.33% <0.00%> (+2.66%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@yurishkuro
Copy link
Member

lgtm, any concerns from your side? Any additional testing required? Bundle size changes?

@mszabo-wikia
Copy link
Contributor Author

The bundle size should be unaffected (seems like it's indeed 1648 kB both on main and this branch). I think overall this should be a fairly safe change as the TS upgrade didn't necessitate widespread code changes.

@yurishkuro yurishkuro merged commit ee7ce64 into jaegertracing:main Mar 9, 2023
@yurishkuro
Copy link
Member

👍

Binrix pushed a commit to Binrix/jaeger-ui that referenced this pull request Apr 18, 2023
## Which problem is this PR solving?
- Contributes towards jaegertracing#818

## Short description of the changes
This is a conservative bump to minimize the amount of changes needed in
tooling and dependencies. Version 3.8.3 was chosen because it is the
version that implements support for type-only exports and imports (i.e.
`import type { TFoo } from '@types/foo';`, which is the major source of
incompatibility with upgraded type definitions in newer dependency
versions.

Notable changes:
* As noted in jaegertracing#998, the `@types/react` version used to type-check
`jaeger-ui` is a transitive dependency on version `16.8.7`, rather than
the expected `18`. Unfortunately, both newer `16.x` type definitions as
well as `18.x` type definitions are incompatible with various
dependencies such as `antd`. As a workaround, downgrade the typing
versions for now and add an explicit dependency on them to the project.
Only index.tsx was using a React 18-specific API (createRoot), so
convert it back to JS until the typings can be updated again.
* TypeScript now enforces that `composite` projects must also generate
declaration files, since that's what the project references system uses.
Make it so. This required a small adjustment to the ErrorMessage
component, as the types of its Message and Details sub-components could
not be reflected in this declaration file; this was trivially fixable by
converting them to named exports instead (which is also consistent with
other areas of the codebase).
* Make Plexus a project reference in the root tsconfig, per the
longstanding todo and per the changes above.

Signed-off-by: Máté Szabó <mszabo@fandom.com>
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.

2 participants