Skip to content

Conversation

eipc16
Copy link
Contributor

@eipc16 eipc16 commented Jun 25, 2025

Hey, I just made a Pull Request!

This should fix an issue where new dependencies where not added with backstage:^ protocol, because new yarn hooks were always failing silently. Explanation here: #30083 (comment)

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)
  • All your commits have a Signed-off-by line in the message. (more info)

…ys fall silently

+ existing implementation did not do anything, because executing `getPackageVersion` always resulted in an error that was then silently suppressed

Signed-off-by: eipc16 <pprzemek.312@gmail.com>
@backstage-goalie
Copy link
Contributor

Unnecessary Changesets

The following package(s) are private and do not need a changeset:

  • yarn-plugin-backstage

Changed Packages

Package Name Package Path Changeset Bump Current Version
yarn-plugin-backstage packages/yarn-plugin patch v0.0.6

Comment on lines -34 to +35
try {
await getPackageVersion(toDescriptor, workspace.project.configuration);
console.warn(
`${toDescriptor.name} should be set to "${PROTOCOL}^" instead of "${toDescriptor.range}". Make sure this change is intentional and not a mistake.`,
);
} catch (_error: any) {
// if there's no found version then this is likely a deprecated package
// or otherwise the plugin won't be able to resolve the real version
// and we should not warn them.
}
console.warn(
`${toDescriptor.name} should be set to "${PROTOCOL}^" instead of "${toDescriptor.range}". Make sure this change is intentional and not a mistake.`,
);
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 was not sure what do to do with it tbh? Previous implementation did not make much sense and the descriptor for the hook should only display a warning... so now it does

@eipc16 eipc16 marked this pull request as ready for review June 25, 2025 11:44
@eipc16 eipc16 requested a review from a team as a code owner June 25, 2025 11:44
@eipc16 eipc16 requested a review from Rugvip June 25, 2025 11:44
Copy link
Member

@freben freben left a comment

Choose a reason for hiding this comment

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

The changes seem to make sense to me, but that's coming from someone who's not intimately familiar with yarn plugin development. @benjdlambert looking good to 🚢 ?

Copy link
Member

@benjdlambert benjdlambert left a comment

Choose a reason for hiding this comment

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

Looks good to me, just probably @freben's comment 🙏

…alled in tests


Update packages/yarn-plugin/src/handlers/afterWorkspaceDependencyAddition.test.ts

Co-authored-by: Fredrik Adelöw <freben@gmail.com>
Signed-off-by: Przemek <32961666+eipc16@users.noreply.github.com>
Copy link
Member

@freben freben left a comment

Choose a reason for hiding this comment

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

Alright, let's try it!

@freben freben merged commit ad0f8af into backstage:master Jun 26, 2025
23 checks passed
Copy link
Contributor

Thank you for contributing to Backstage! The changes in this pull request will be part of the 1.41.0 release, scheduled for Tue, 15 Jul 2025.

@benjdlambert benjdlambert mentioned this pull request Jul 15, 2025
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