-
Notifications
You must be signed in to change notification settings - Fork 513
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
Typescript version is outdated #818
Labels
Comments
If you have cycles & expertise, go for it. |
@yurishkuro sure, we'll take it |
This was referenced Feb 25, 2023
yurishkuro
added a commit
that referenced
this issue
Mar 7, 2023
## Which problem is this PR solving? - Contributes towards #818, #1199 ## Short description of the changes Switch the project to build using Vite, per the considerations outlined in #1212. Notable changes are: * `react-scripts` would provide some built-in Jest transforms and mocks under the hood, which need to be defined explicitly now. * Build-time injected constants no longer utilize `process.env`. ## Testing For this, I spun up a local `all-in-one` jaeger instance and fed it some test data with `microsim`. I updated the `jaeger-ui` submodule reference for this local jaeger checkout to point to this branch to test the behavior of the production build. I then navigated around looking for errors on the pages or the console. <img width="1512" alt="Screenshot 2023-03-02 at 01 07 39" src="https://user-images.githubusercontent.com/2721291/222296252-539ac6e0-15b5-49d6-943c-e437b852704e.png"> --------- Signed-off-by: Máté Szabó <mszabo@fandom.com> Signed-off-by: Yuri Shkuro <github@ysh.us> Co-authored-by: Yuri Shkuro <github@ysh.us>
yurishkuro
pushed a commit
that referenced
this issue
Mar 9, 2023
## Which problem is this PR solving? - Contributes towards #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 #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>
yurishkuro
pushed a commit
that referenced
this issue
Mar 12, 2023
## Which problem is this PR solving? - Resolves #818 ## Short description of the changes Update TypeScript, ESLint and `@typescript-eslint` to the latest versions. Notable changes: * Update the list of core ESLint rules that need to be disabled because `@typescript-eslint` provides TypeScript-specific equivalents for them, and change existing suppressions referencing the now-disabled core rules. Group these together in the ESLint config for clarity. * Fix references to unimported types in `serviceGraph.tsx` and `types.tsx`. * It seems that the type of `defaultConfig` and the user-defined config wasn't correctly inferred as `Config` before the upgrade. Fix various type errors resulting from this: * `'__mergeFields'`, i.e. the list of config options where user-specified values should be merged with the default object values in `defaultConfig`, was defined via `defineProperty` and so seemingly inferred as an unknown type. This now causes a type error when iterating over its keys to try and merge the relevant property values, as TypeScript can't guarantee that the property values are objects. Make it a separate named export instead, which also allows for strictly typing the allowable property names. * Make the `url` field of the `ConfigMenuItem` type optional as this was now recognized as a type error. This matches documentation and actual usage. * Rename the `dagMaxServicesLen` property definition to `dagMaxNumServices` as that is what the actual code uses. * Fix the `linkPatterns` config option typedef to be an array, as per actual usage. * Update `handleError` in Plexus to correctly take either an `ErrorEvent` or a `MessageEvent` to match updated typings for the Web Worker API. * Update `catch` clauses in `readJsonFile` because TypeScript 4.4+ [types the error in `catch` clauses as `unknown` by default](https://devblogs.microsoft.com/typescript/announcing-typescript-4-4/#using-unknown-in-catch-variables). We don't expect anything outside of an `Error` to be thrown here, so we can restore the previous behavior. Signed-off-by: Máté Szabó <mszabo@fandom.com>
Binrix
pushed a commit
to Binrix/jaeger-ui
that referenced
this issue
Apr 18, 2023
## Which problem is this PR solving? - Contributes towards jaegertracing#818, jaegertracing#1199 ## Short description of the changes Switch the project to build using Vite, per the considerations outlined in jaegertracing#1212. Notable changes are: * `react-scripts` would provide some built-in Jest transforms and mocks under the hood, which need to be defined explicitly now. * Build-time injected constants no longer utilize `process.env`. ## Testing For this, I spun up a local `all-in-one` jaeger instance and fed it some test data with `microsim`. I updated the `jaeger-ui` submodule reference for this local jaeger checkout to point to this branch to test the behavior of the production build. I then navigated around looking for errors on the pages or the console. <img width="1512" alt="Screenshot 2023-03-02 at 01 07 39" src="https://user-images.githubusercontent.com/2721291/222296252-539ac6e0-15b5-49d6-943c-e437b852704e.png"> --------- Signed-off-by: Máté Szabó <mszabo@fandom.com> Signed-off-by: Yuri Shkuro <github@ysh.us> Co-authored-by: Yuri Shkuro <github@ysh.us>
Binrix
pushed a commit
to Binrix/jaeger-ui
that referenced
this issue
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>
Binrix
pushed a commit
to Binrix/jaeger-ui
that referenced
this issue
Apr 18, 2023
- Resolves jaegertracing#818 Update TypeScript, ESLint and `@typescript-eslint` to the latest versions. Notable changes: * Update the list of core ESLint rules that need to be disabled because `@typescript-eslint` provides TypeScript-specific equivalents for them, and change existing suppressions referencing the now-disabled core rules. Group these together in the ESLint config for clarity. * Fix references to unimported types in `serviceGraph.tsx` and `types.tsx`. * It seems that the type of `defaultConfig` and the user-defined config wasn't correctly inferred as `Config` before the upgrade. Fix various type errors resulting from this: * `'__mergeFields'`, i.e. the list of config options where user-specified values should be merged with the default object values in `defaultConfig`, was defined via `defineProperty` and so seemingly inferred as an unknown type. This now causes a type error when iterating over its keys to try and merge the relevant property values, as TypeScript can't guarantee that the property values are objects. Make it a separate named export instead, which also allows for strictly typing the allowable property names. * Make the `url` field of the `ConfigMenuItem` type optional as this was now recognized as a type error. This matches documentation and actual usage. * Rename the `dagMaxServicesLen` property definition to `dagMaxNumServices` as that is what the actual code uses. * Fix the `linkPatterns` config option typedef to be an array, as per actual usage. * Update `handleError` in Plexus to correctly take either an `ErrorEvent` or a `MessageEvent` to match updated typings for the Web Worker API. * Update `catch` clauses in `readJsonFile` because TypeScript 4.4+ [types the error in `catch` clauses as `unknown` by default](https://devblogs.microsoft.com/typescript/announcing-typescript-4-4/#using-unknown-in-catch-variables). We don't expect anything outside of an `Error` to be thrown here, so we can restore the previous behavior. 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
Requirement - what kind of business use case are you trying to solve?
We'd like to upgrade the typescript version from 3.5.3 to the latest one (4.4)
Problem - what in Jaeger blocks you from solving the requirement?
the current version is missing key features
Related: #998
The text was updated successfully, but these errors were encountered: