Upgrades TypeScript to 4.1, simplifies jest config #1230
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tmarkley
added
dependencies
AMoo-Miki
pushed a commit
to AMoo-Miki/OpenSearch-Dashboards
that referenced
this pull request
Feb 10, 2022
AMoo-Miki
pushed a commit
to AMoo-Miki/OpenSearch-Dashboards
that referenced
this pull request
Feb 10, 2022
# [31.1.0](elastic/elastic-charts@v31.0.0...v31.1.0) (2021-07-06) ### Bug Fixes * **heatmap:** pick correct brush end value ([opensearch-project#1230](elastic/elastic-charts#1230)) ([cb95a75](elastic/elastic-charts@cb95a75)), closes [opensearch-project#1229](elastic/elastic-charts#1229) ### Features * **a11y:** accessible goal and gauge chart ([#1174](elastic/elastic-charts#1174)) ([775dc98](elastic/elastic-charts@775dc98)), closes [opensearch-project#1160](elastic/elastic-charts#1160)
|
I'm taking a look at the error that the functional tests are throwing: |
7 tasks
|
Currently trying to fix the bootstrap errors related to |
Would this reduce the delta changed in this PR but resolve the CVE if the merging happened in a separate PR? |
Yeah, but I can't get the TypeScript upgrade to bootstrap even without those jest changes. There's a conflict with the jest dependencies so I'm going to wait for #1301 and then try again. |
tmarkley
force-pushed
the
nth-check
branch
2 times, most recently
from
3b5c51a to
5027633
Compare
ashwin-pc
previously approved these changes
Mar 4, 2022
| @@ -135,7 +135,7 @@ describe('Server logging configuration', function () { | |||
| '--verbose', | |||
| ]); | |||
|
|
|||
| const message$ = Rx.fromEvent(child.stdout, 'data').pipe( | |||
| const message$ = Rx.fromEvent(child.stdout!, 'data').pipe( | |||
There was a problem hiding this comment.
Nit: This is nullable. Given that its a test it should be okay though.
tmarkley
force-pushed
the
nth-check
branch
2 times, most recently
from
cdc3585 to
7c1155d
Compare
tmarkley
force-pushed
the
nth-check
branch
3 times, most recently
from
bf318d4 to
1f3399a
Compare
tmarkley
force-pushed
the
nth-check
branch
2 times, most recently
from
e35cbfa to
f44d550
Compare
|
Did we want to put this PR in draft? |
Good question. I'm still actively trying to fix the functional test error, but I guess I can put it in draft until that's fixed. |
|
Looks like the CVE was addressed in this PR. For that reason and the conflicts, I will remove the |
* Addressing the `nth-check` CVE requires bumping `css-select`, which is a dependency of `cheerio`. Bumping `cheerio` requires upgrading from TypeScript 4.0 to 4.1. * TypeScript 4.1 introduces a set of [breaking changes](https://devblogs.microsoft.com/typescript/announcing-typescript-4-1/#breaking-changes). The main changes that impact Dashboards is that `resolve`'s parameters are no longer optional in `Promise`s, and that potentially undefined indexes must use the `!` non-null assertion operator. * The upgrades to TypeScript and `cheerio` triggered some jest errors which prompted the upgrade to the `enzyme` dependencies. * Merges files under `/src/test_utils` and `/src/dev/jest` into the `@osd/test` package to simplify. * Fixes the naming of the `@osd/eslint-config-opensearch-dashboards` package. * Fixes inconsistent plugin installation tests. Resolves opensearch-project#1081 Signed-off-by: Tommy Markley <[email protected]>
|
Verify impact to plugins. |
|
Closing. We are not doing a TypeScript upgrade now as the refactor is too much for the benefit. We can revisit this later. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Most of the PR is just adding parameters to
Promises and changing references to the updated@osd/test/jestdirectory.nth-checkCVE required bumpingcss-select, which is a dependency ofcheerio. Bumpingcheeriorequires upgrading from TypeScript 4.0 to 4.1.resolve's parameters are no longer optional inPromises, and that potentially undefined indexes must use the!non-null assertion operator.cheeriotriggered some jest errors which prompted the upgrade to theenzymedependencies./src/test_utilsand/src/dev/jestinto the@osd/testpackage to simplify.@osd/eslint-config-opensearch-dashboardspackage.Issues Resolved
Resolves #1081
Partially addresses #1187
Check List
yarn test:jestyarn test:jest_integrationyarn test:ftr