chore: upgrade to typescript 4.8 #3875

candrews · 2022-09-27T14:58:57Z

Follows CONTRIBUTING rules

What does this PR do?

Upgrade typescript from 3.9.10 to 4.8.3

Includes necessary upgrades to these packages as well: @typescript-eslint/eslint-plugin
@typescript-eslint/parser
eslint
eslint-config-prettier
eslint-plugin-jest
jest
tap
ts-jest
ts-loader
ts-node

Where should the reviewer start?

There's no functional change here. The only changes made are the results newly identified issues from the upgraded typescript and eslint.

How should this be manually tested?

Run snyk :)

Any background context you want to provide?

Using the latest versions of dependencies is a general best practice for a number of reasons.

My primary motivation here is that I want to add a feature to snyk cli (for which I will submit a pull request in the future) that uses a dependency that's written in Typescript 4.8, and Typescript 3.9 (which snyk cli currently uses) cannot compile against it.

What are the relevant tickets?

Screenshots

Additional questions

CLAassistant · 2022-09-27T14:59:06Z

All committers have signed the CLA.

candrews · 2022-09-28T20:23:14Z

@magdziarek could you please take a look? Thank you!

candrews · 2022-09-30T16:50:32Z

I just rebased this again...

@novalex thank you for the approval!

@teodora-sandu @jonathansantilli @admons @tonidevine1 and anyone else - Is there anything I can do to move this along?

PeterSchafer · 2022-10-07T09:19:21Z

Dear @candrews,
thank you very much for your contribution. We have reviewed your changes and they generally look good. Unfortunately they cause tests to fail.
You should be able to reproduce this by running npm run test while some tests might locally fail due to missing dependencies, some failures are related to the changes in the PR itself.
Thanks again!

candrews · 2022-10-07T12:17:09Z

Thanks for this information and for enabling workflows! I'll get right on figuring out and addressing these issues.

candrews · 2022-10-07T14:21:45Z

@PeterSchafer I've updated this PR - all failures have been fixed.

When I run npm test now, I get the same result as when I run npm test against master, which is that these 4 tests fail:

Summary of all failing tests
 FAIL  test/jest/unit/lib/ecosystems/resolve-test-facts.spec.ts
  ● resolve and test facts › successfully resolving and testing file-signatures fact for c/c++ projects with new unmanaged service

    getaddrinfo ENOTFOUND api.example.com



  ● resolve and test facts › failed resolving and testing file-signatures since createDepGraph throws exception with new unmanaged service

    getaddrinfo ENOTFOUND api.example.com



  ● resolve and test facts › failed resolving and testing file-signatures since getDepGraph throws exception with new unmanaged service

    getaddrinfo ENOTFOUND api.example.com



  ● resolve and test facts › failed resolving and testing file-signatures since getIssues throws exception with new unmanaged service

    getaddrinfo ENOTFOUND api.example.com

Thank you again for taking a look, approving the workflow, and providing feedback. I believe your concerns have now been addressed, so hopefully this is now ready to merge.

candrews · 2022-10-11T14:07:31Z

@PeterSchafer I updated this PR fixing the error in test/jest/acceptance/config.spec.ts that the test_and_release workflow discovered (npm test didn't report this error, so I didn't know about it 🤷). I also rebased it against master and resolved conflicts.

PeterSchafer · 2022-10-11T16:14:09Z

@candrews Thanks for the additional changes. I just triggered test_and_release again and it continues to fail. First because of a linting issue, second due to other tests still failing. Could you please run npm run format before a commit.
The failing tests should be reproducible when running the individual test targets like npm run test:unit. I hope this helps.

candrews · 2022-10-11T17:18:52Z

Could you please run npm run format before a commit.

I'm sorry I forgot to do that :(

The failing tests should be reproducible when running the individual test targets like npm run test:unit.

I fixed the "tap" test compilation errors. I get failures in the tests themselves, but I'm hoping that's due to my system's setup and that they pass when run in the workflow environment.

The node 12 tests is failing because Typescript no longer Node 12. Node 12 has been end of life and out of support since 30 Apr 2022 (5 months ago). Therefore I suggest Snyk stop running tests with it; I've added a commit to do so.

I don't know why the 2 windows tests, v2 / Jest Acceptance Tests (windows/amd64) and Acceptance Tests (snyk-win.exe), are failing. The tests don't have any output that indicates what the problem is. If you give me some hints as to what the problem is or how I can reproduce it, I'd be happy to do my best to fix it.

The output can contain ansi escape sequences which must be stripped to ensure consistent test behavior.

node-tap automatically runs tests in parallel instead of one at at time. See: https://node-tap.org/changelog/#130---2019-04-25 node-tap 15.0 changed the behavior of teardown, making it asynchronous. See: https://node-tap.org/changelog/#150---2021-03-30 Therefore one tap tests may be running while another runs or while another has finished running but hasn't finished executing teardown. With all tests using the same port, 12345, multiple tests would try to start a server on that port at the same time resulting it tests randomly failing with: listen EADDRINUSE: address already in use :::12345 Using a different port per process eliminates this problem. See: https://node-tap.org/docs/api/parallel-tests/

Since tap tests can run in parallel, if multiple tap tests use the same configuration directory and they try to update configuration, they'll overwrite each other and end up in indeterminate states, causing unexpected test failures.

Node 12 went end of life on April 30, 2022. See: https://twitter.com/nodejs/status/1524081123579596800 See: https://github.com/nodejs/Release#release-schedule Signed-off-by: Craig Andrews <candrews@integralblue.com>

Upgrade typescript from 3.9.10 to 4.8.4 Includes necessary upgrades to these packages as well: @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint eslint-config-prettier eslint-plugin-jest jest tap ts-jest ts-loader ts-node Signed-off-by: Craig Andrews <candrews@integralblue.com>

Solves error when using node 17 (and later): Error: error:0308010C:digital envelope routines::unsupported See: webpack/webpack#14532

candrews · 2022-10-14T20:14:20Z

I found (and fixed) more race conditions in the tap tests. Can you please run the workflow so can I see if there's anything else wrong?

I continue to not know why the Windows tests faill; I am eager to get whatever information you can share about them.

candrews · 2022-10-16T20:48:28Z

**code/snyk (Snyk Internal (Org)) ** — 9 new Code Analysis issues found Details

I have no idea what this means. And the link goes to an access denied page.

novalex · 2022-10-17T17:27:52Z

I have no idea what this means. And the link goes to an access denied page.

That is a check for newly introduced vulnerabilities, but the 9 detected issues are just because of the test file changes and not actually new issues so have bypassed it.

candrews · 2022-10-17T19:44:57Z

I broke this PR up into a few smaller ones that are easier to review in hope of making at least some progress 🤞

ipapast · 2022-11-09T15:26:58Z

is this PR still relevant @candrews? or can we close it?

candrews · 2022-11-09T15:41:07Z

It's still relevant. Snyk's typescript dependency is still out of date and per Snyk's own documentation, having out of date dependencies is problematic for maintainbility as well as security.

ipapast · 2022-11-09T15:48:26Z

It's still relevant. Snyk's typescript dependency is still out of date and per Snyk's own documentation, having out of date dependencies is problematic for maintainbility as well as security.

okay then, I thought this would be done in more, smaller PRs from the comment here #3875 (comment)

candrews · 2022-11-09T17:35:20Z

I'd love to get this done in whatever works for Snyk - big PR or small PRs, whatever we can actually do to make progress would be great from my perspective.

If Snyk wants to do smaller PRs, can the workflows please be run for them, and then we can merge them? #4147 might be a nice starting point.

ipapast · 2022-11-09T17:42:57Z

I'd love to get this done in whatever works for Snyk - big PR or small PRs, whatever we can actually do to make progress would be great from my perspective.

If Snyk wants to do smaller PRs, can the workflows please be run for them, and then we can merge them? #4147 might be a nice starting point.

no problem by me, I was only going through the pull request reviews pending, and noticed the comment I mentioned. feel free to work in any way you prefer :)

thisislawatts · 2024-03-08T12:30:12Z

This upgrade has now been implemented as part of #5046

candrews requested review from a team as code owners September 27, 2022 14:58

candrews requested review from teodora-sandu, jonathansantilli, admons and tonidevine1 September 27, 2022 14:58

candrews force-pushed the chore/typescript4.8 branch from cee4945 to 35551c8 Compare September 29, 2022 16:23

novalex previously approved these changes Sep 30, 2022

View reviewed changes

candrews force-pushed the chore/typescript4.8 branch 2 times, most recently from 590c752 to 695699e Compare September 30, 2022 17:05

candrews mentioned this pull request Oct 1, 2022

feat: add "sbom" command that produces a CycloneDX 1.4 JSON SBOM #3983

Closed

1 task

candrews force-pushed the chore/typescript4.8 branch from 695699e to ea11ba2 Compare October 3, 2022 13:11

candrews force-pushed the chore/typescript4.8 branch 2 times, most recently from 9b345d6 to b34f84a Compare October 7, 2022 14:17

candrews mentioned this pull request Oct 11, 2022

chore: upgrade to typescript 4.8 #4115

Closed

1 task

candrews force-pushed the chore/typescript4.8 branch from b34f84a to 8ec1309 Compare October 11, 2022 14:06

candrews force-pushed the chore/typescript4.8 branch from 8ec1309 to 8709316 Compare October 11, 2022 17:18

candrews requested a review from a team as a code owner October 11, 2022 17:18

candrews force-pushed the chore/typescript4.8 branch 2 times, most recently from 825ddc1 to 4856ab8 Compare October 13, 2022 23:45

candrews mentioned this pull request Oct 14, 2022

[Snyk-dev] Upgrade typescript from 4.3.2 to 4.8.3 #4136

Closed

candrews force-pushed the chore/typescript4.8 branch 3 times, most recently from a3fda59 to fff6c57 Compare October 14, 2022 18:52

candrews added 7 commits October 14, 2022 16:11

fix: stripAnsi in test/tap/display-test-results.test.ts

fc0f905

The output can contain ansi escape sequences which must be stripped to ensure consistent test behavior.

fix: test/tap/auth.test.ts: await server.close in teardown

c506a75

chore: upgrade node to 14; node 12 is end of life

55f428f

Node 12 went end of life on April 30, 2022. See: https://twitter.com/nodejs/status/1524081123579596800 See: https://github.com/nodejs/Release#release-schedule Signed-off-by: Craig Andrews <candrews@integralblue.com>

chore: upgrade webpack to 5.74.0

e72d1fa

Solves error when using node 17 (and later): Error: error:0308010C:digital envelope routines::unsupported See: webpack/webpack#14532

candrews force-pushed the chore/typescript4.8 branch from fff6c57 to e72d1fa Compare October 14, 2022 20:11

This was referenced Oct 17, 2022

fix: Tap Test improvements/fixes #4145

Closed

chore: upgrade node to 14; node 12 is end of life #4146

Closed

chore: upgrade webpack to 5.74.0 #4147

Open

teodora-sandu removed their request for review November 4, 2022 12:42

bastiandoetsch dismissed novalex’s stale review via e72d1fa February 15, 2023 13:26

thisislawatts closed this Mar 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: upgrade to typescript 4.8 #3875

chore: upgrade to typescript 4.8 #3875

candrews commented Sep 27, 2022 •

edited

CLAassistant commented Sep 27, 2022 •

edited

candrews commented Sep 28, 2022

candrews commented Sep 30, 2022

PeterSchafer commented Oct 7, 2022

candrews commented Oct 7, 2022

candrews commented Oct 7, 2022

candrews commented Oct 11, 2022 •

edited

PeterSchafer commented Oct 11, 2022

candrews commented Oct 11, 2022

candrews commented Oct 14, 2022

candrews commented Oct 16, 2022

novalex commented Oct 17, 2022

candrews commented Oct 17, 2022

ipapast commented Nov 9, 2022

candrews commented Nov 9, 2022

ipapast commented Nov 9, 2022

candrews commented Nov 9, 2022

ipapast commented Nov 9, 2022

thisislawatts commented Mar 8, 2024

chore: upgrade to typescript 4.8 #3875

chore: upgrade to typescript 4.8 #3875

Conversation

candrews commented Sep 27, 2022 • edited

What does this PR do?

Where should the reviewer start?

How should this be manually tested?

Any background context you want to provide?

What are the relevant tickets?

Screenshots

Additional questions

CLAassistant commented Sep 27, 2022 • edited

candrews commented Sep 28, 2022

candrews commented Sep 30, 2022

PeterSchafer commented Oct 7, 2022

candrews commented Oct 7, 2022

candrews commented Oct 7, 2022

candrews commented Oct 11, 2022 • edited

PeterSchafer commented Oct 11, 2022

candrews commented Oct 11, 2022

candrews commented Oct 14, 2022

candrews commented Oct 16, 2022

novalex commented Oct 17, 2022

candrews commented Oct 17, 2022

ipapast commented Nov 9, 2022

candrews commented Nov 9, 2022

ipapast commented Nov 9, 2022

candrews commented Nov 9, 2022

ipapast commented Nov 9, 2022

thisislawatts commented Mar 8, 2024

candrews commented Sep 27, 2022 •

edited

CLAassistant commented Sep 27, 2022 •

edited

candrews commented Oct 11, 2022 •

edited