Skip to content

Upgrade dependencies #1128

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

Merged
merged 2 commits into from
May 18, 2023
Merged

Conversation

nlebrun-spotify
Copy link
Contributor

@nlebrun-spotify nlebrun-spotify commented May 15, 2023

BREAKING CHANGE: TypeScript bump from v4 to v5, Jest bump from v28 to v29

Upgraded most of the dependencies, except the ones that start publishing ESM-only versions since I didn't wanted to revamp the build system to support these.

It also solve a security issue with yaml which had an outdated version required by some outdated dependencies.

Unverified

This user has not yet uploaded their public signing key.
BREAKING CHANGE: TypeScript bump from v4 to v5, Jest bump from v28 to v29
"lerna": "^5.5.2",
"typescript": "^4.5.0"
},
"resolutions": {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubled-checked, these two packages (minimist and ansi-regex) now have higher versions than these in the dependency tree, so these resolutions to fix vulnerabilities are not needed anymore

},
"peerDependencies": {
"@spotify/eslint-plugin": ">=8.x",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's already in the dependencies

@@ -2,6 +2,7 @@
"name": "@spotify/web-scripts-utils",
"version": "14.1.6",
"description": "Private package which contains re-used utils within web-scripts projects",
"license": "Apache-2.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added missing license, it was throwing a warning

'ts-jest': {
tsconfig: {
allowJs: true,
transform: {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the new pattern for ts-jest configuration, the globals options are deprecated and emit a warning

},
"devDependencies": {
"@types/rimraf": "^3.0.0",
"@types/tempy": "^0.3.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tempy already provides its own types

@@ -285,7 +285,7 @@ function handleSpawnResult(result: SpawnSyncReturns<Buffer>) {
}

function getCommand(args: any[]): Command {
return args[0] as Command;
return args[1] as Command;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the breaking change of commander was that actions now return [parameter, options, command] instead of [parameter, command]

@@ -130,7 +130,6 @@ describe.skip('integration tests', () => {
'typescript',
'@types/jest',
'@types/react',
'@types/react-dom',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

react-dom was unused within the template, and its types were colliding with other types, causing issues

@javoire
Copy link
Member

javoire commented May 15, 2023

Great! We'll release this as a major version of web-scripts to highlight the breaking changes

@javoire
Copy link
Member

javoire commented May 15, 2023

@nlebrun-spotify could you set this to [18] ? I don't think I have write access to your fork

@javoire
Copy link
Member

javoire commented May 15, 2023

We might as well add this to our own package.json as well to make that requirement clearer to our users

@nlebrun-spotify
Copy link
Contributor Author

Great! We'll release this as a major version of web-scripts to highlight the breaking changes

Yep, I added the semantic-release BREAKING CHANGE keyword in the commit message so it will run as a major release

Unverified

This user has not yet uploaded their public signing key.
BREAKING CHANGE: Node bump from v14 to v18
@nlebrun-spotify
Copy link
Contributor Author

nlebrun-spotify commented May 16, 2023

We might as well add this to our own package.json as well to make that requirement clearer to our users

I upgraded every reference of Node version to 18, very good catch!
Also changed the CI to run tests only on Node 18 (it was throwing in lower versions due to semantic-release requirements)

@javoire javoire merged commit 6519dca into spotify:master May 18, 2023
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.

None yet

2 participants