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

Update all non major NPM dependencies #127

Merged
merged 2 commits into from
Sep 19, 2023
Merged

Conversation

@renovate renovate bot requested a review from a team as a code owner September 11, 2023 14:31
@renovate renovate bot changed the title Update dependency typescript to ~5.2.2 Update all non major NPM dependencies Sep 11, 2023
@renovate renovate bot force-pushed the renovate-all-npm-minor-patch branch 14 times, most recently from 480fe4e to 36d4817 Compare September 18, 2023 08:21
@xoen
Copy link
Contributor

xoen commented Sep 18, 2023

(As commented in ministryofjustice/hmpps-incentives-ui#602 (comment))

Looks like tests fail with a bunch of the following error:

error TS5110: Option 'module' must be set to 'Node16' when option 'moduleResolution' is set to 'Node16'. https://github.com/kulshekhar/ts-jest/issues/4198 or to do with the new version of TS.

I played with this locally and it looks explicitly set module/moduleResolution as follow in the tsconfig.json solves the issue:

"module": "CommonJS",
"moduleResolution": "Node",

I don't think this changes anything in practice as at the moment it's a node app using CommonJS modules but do we think there is any drawback of adding these to the tsconfig.json?

@renovate renovate bot force-pushed the renovate-all-npm-minor-patch branch from 36d4817 to 5700263 Compare September 18, 2023 23:38
Setting these explicitly avoids these errors:

```
Option 'module' must be set to 'Node16' when option 'moduleResolution' is set to 'Node16'.
```
@xoen xoen requested a review from ushkarev September 19, 2023 10:57
@xoen
Copy link
Contributor

xoen commented Sep 19, 2023

@ushkarev with this change tests pass, should we go for it so that renovate is finally happy?

AFAICT this change shouldn't change anything in practice (but there is some bug in jest or ts-jest or typescript which causes these errors)

@ushkarev ushkarev merged commit 2b94ec7 into main Sep 19, 2023
4 checks passed
@ushkarev ushkarev deleted the renovate-all-npm-minor-patch branch September 19, 2023 11:01
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