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

feat: support React 18; drop support for Paragon v21 in favor of v22 #441

Merged
merged 5 commits into from
Feb 26, 2025

Conversation

adamstankiewicz
Copy link
Member

@adamstankiewicz adamstankiewicz commented Feb 26, 2025

BREAKING CHANGE: drops support for @openedx/paragon v21 in favor of v22.

feat: support React 18


Fixes Pagination style bug in search:

image

Merge checklist:

  • Evaluate how your changes will impact existing consumers (e.g., frontend-app-learner-portal-enterprise, frontend-app-admin-portal, and frontend-app-enterprise-public-catalog). Will consumers safely be able to upgrade to this change without any breaking changes?
  • Ensure your commit message follows the semantic-release conventional commit message format. If your changes include a breaking change, ensure your commit message is explicitly marked as a BREAKING CHANGE so the NPM package is released as such.
  • Once CI is passing, verify the package versions that Lerna will increment to in the Github Action CI workflow logs.
    • Note: This may be found in the "Preview Updated Versions (dry run)" step in the Github Action CI workflow logs.

Post merge:

  • Follow the release steps in the README documentation. Verify Lerna's release commit (e.g., chore(release): publish new versions) that incremented versions in relevant package.json and CHANGELOG files, and created Git tags for those versions is on master (Important: ensure the Git tags are for the correct commit SHA).
  • Run the Publish from package.json Github Action workflow to publish these new package versions to NPM.
    • This may be triggered by clicking the "Run workflow" option for the master branch.
  • Verify the new package versions were published to NPM (i.e., npm view <package_name> versions --json).
    • Note: There may be a slight delay between when the workflow finished and when NPM reports the package version as being published. If it doesn't appear right away in the above command, try again in a few minutes.

@adamstankiewicz adamstankiewicz force-pushed the ags/catalog-search-react-18 branch from daec889 to e33ca7a Compare February 26, 2025 15:08
@adamstankiewicz adamstankiewicz changed the title feat: upgrade to paragon v22 and react v18 feat: support React 18; drop support for Paragon v21 in favor of v22 Feb 26, 2025
Copy link

codecov bot commented Feb 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.94%. Comparing base (f48e3e6) to head (6754f05).
Report is 1 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #441      +/-   ##
==========================================
- Coverage   78.99%   78.94%   -0.06%     
==========================================
  Files          37       37              
  Lines         757      755       -2     
  Branches      191      191              
==========================================
- Hits          598      596       -2     
  Misses        146      146              
  Partials       13       13              
Files with missing lines Coverage Δ
packages/catalog-search/src/SearchPagination.jsx 100.00% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f48e3e6...6754f05. Read the comment docs.

@adamstankiewicz adamstankiewicz force-pushed the ags/catalog-search-react-18 branch from fa4d6ad to fededc1 Compare February 26, 2025 17:28
BREAKING CHANGE: drop support for Paragon v21 in `@edx/frontend-enterprise-catalog-search`
@adamstankiewicz adamstankiewicz force-pushed the ags/catalog-search-react-18 branch from fededc1 to 5c178fe Compare February 26, 2025 17:28
Copy link
Member

@brobro10000 brobro10000 left a comment

Choose a reason for hiding this comment

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

1 non blocking nit. Great job

@@ -1,7 +1,7 @@
import React, { useContext, useMemo } from 'react';
import React, { useContext } from 'react';
Copy link
Member

Choose a reason for hiding this comment

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

[nit/non-blocking] Do we still need to import React. If not is it a good opportunity to stop importing it if we don't need it.

Copy link
Member Author

@adamstankiewicz adamstankiewicz Feb 26, 2025

Choose a reason for hiding this comment

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

Good suggestion, but if I change it here, we should just change it everywhere. I also recall during development where I did try to remove it in a few places while working on the PR, but it complained (at least in tests) that React couldn't be found without it imported. Will defer for separate contribution/fix.

@@ -20,7 +20,7 @@
"build": "make build",
"i18n_extract": "BABEL_ENV=i18n fedx-scripts babel src --quiet > /dev/null",
"lint": "fedx-scripts eslint --ext .js --ext .jsx --ext .ts --ext .tsx .",
"lint:fix": "npm run test -- --fix",
"lint:fix": "npm run lint -- --fix",
Copy link
Member

Choose a reason for hiding this comment

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

🥳

"@openedx/paragon": "^21.0.0",
"react": "^16.12.0 || ^17.0.0",
"react-dom": "^16.12.0 || ^17.0.0",
"@openedx/paragon": "^22.0.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

[inform] here is the breaking change

Comment on lines +70 to +71
leftIcon: ArrowBackIos,
rightIcon: ArrowForwardIos,
Copy link
Member Author

Choose a reason for hiding this comment

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

[inform] addressing Paragon v22 breaking change

@adamstankiewicz adamstankiewicz merged commit 7046540 into master Feb 26, 2025
9 checks passed
@adamstankiewicz adamstankiewicz deleted the ags/catalog-search-react-18 branch February 26, 2025 20:13
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