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

fix: change Paragon to a peerDependency #425

Merged
merged 2 commits into from
Jan 31, 2025
Merged

Conversation

MaxFrank13
Copy link
Member

When carrying over some changes from the OpenedX version of the footer, we changed Paragon from a peerDependency to a regular dependency (source). The Open edX footer has since fixed this and correctly has Paragon set to a peerDependency (source).

Paragon should be a peerDependency here so that users don't end up having to download multiple versions of Paragon and we don't introduce any style regressions.

"js-cookie": "^3.0.1",
"lodash": "^4.17.21"
},
"peerDependencies": {
"@edx/frontend-platform": "^7.0.0 || ^8.0.0",
"@openedx/paragon": ">= 21.11.3 < 23.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

[sanity check] is there anything in the major upgrade from v21 to v22 that could break frontend-component-footer-edx? That is, will supporting an older v21 version be compatible given it was previously using v22? If not, then I agree, expanding the support back to v21 is reasonable :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh good call out. I was just replicating what was done in the Open edX version. This at least gives some reassurance. But looking at the breaking changes for v22, I don't see anything in there that would cause footer to break. The components called out in the release notes aren't used by frontend-component-footer-edx.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for verifying!

@MaxFrank13 MaxFrank13 force-pushed the mfrank/remove-dependency branch from 88f9f15 to bf9aa64 Compare January 31, 2025 14:24
@MaxFrank13 MaxFrank13 merged commit 4a8395f into master Jan 31, 2025
3 checks passed
@MaxFrank13 MaxFrank13 deleted the mfrank/remove-dependency branch January 31, 2025 14:30
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