Skip to content

Conversation

@Mrtenz
Copy link
Member

@Mrtenz Mrtenz commented Apr 3, 2023

Description

This pull request modifies the getUpdatedDependencyField function to avoid overriding the dependencies that should not
be updated. The update process should ignore certain dependencies that are used across multiple packages in a workspace.

The change involves introducing a check for the presence of workspace:^ in the package.json file before updating
the version. This is an important step because workspace:^ should not be overridden with new version ranges. The rest
of the dependency versions can be updated, however.

Changes

  1. Added a check for workspace:^ in getUpdatedDependencyField that prevents updating the version range for this dependency.

…version

This change modifies getUpdatedDependencyField function to check the dependencyObject values for `workspace:^`
as a condition before replacing the version with newVersionRange. The 'workspace:^' should not be overridden with
newVersionRange.
@Mrtenz Mrtenz requested a review from a team as a code owner April 3, 2023 13:58
Without specifying a version in the workflow, the job could fail or use the wrong version of Node.js. This change ensures that the job uses the Node.js version specified in .nvmrc.
Comment on lines +13 to +15
- uses: actions/setup-node@v3
with:
node-version-file: '.nvmrc'
Copy link
Member Author

Choose a reason for hiding this comment

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

This package never had a proper Node.js setup step. The default Node.js version now seems to be 18, causing installation of dependencies to fail.

Gudahtt
Gudahtt previously approved these changes Apr 3, 2023
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

: dependencyObject[packageName];
newDeps[packageName] =
packagesToUpdate.has(packageName) &&
dependencyObject[packageName] !== 'workspace:^'
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Perhaps we could see if it starts with workspace: instead? We usually use the ^ range but the problem should apply for any type of range used.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, updated!

This change checks whether a dependency field starts with 'workspace:', instead of being equal to 'workspace:^'.
@Mrtenz Mrtenz merged commit e07d39e into main Apr 3, 2023
@Mrtenz Mrtenz deleted the mrtenz/support-workspace-dependencies branch April 3, 2023 15:23
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.

3 participants