Skip to content

Conversation

feanil
Copy link
Contributor

@feanil feanil commented Sep 23, 2025

Remove @edx/reactifex package from devDependencies as it is no longer
needed. Translation extraction functionality has been verified to work
correctly without this dependency.

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

@codecov
Copy link

codecov bot commented Sep 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.14%. Comparing base (b106d06) to head (1c9e20e).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #256   +/-   ##
=======================================
  Coverage   84.14%   84.14%           
=======================================
  Files          47       47           
  Lines         700      700           
  Branches      135      135           
=======================================
  Hits          589      589           
  Misses        111      111           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@feanil feanil force-pushed the feanil/remove-reactifex-packages branch from 4b4bc6e to 95eb4d2 Compare September 24, 2025 14:16
@feanil
Copy link
Contributor Author

feanil commented Sep 24, 2025

@openedx/committers-frontend Can you check my approach on this, when I updated the package.json and re-built package-lock.json for a different change (dropping reactifex), it pulled in a new version of frontend-component-header which now has typescript components. Adding the tsconfig and updating the eslint ignore seems to have gotten the build passing but I wanted to make sure it's the correct approach before I apply the changes to other related PRs that are seeing this issue.

@brian-smith-tcril
Copy link
Contributor

These changes look similar to the ones made in #254

It seems openedx/frontend-component-header#604 should have landed as a breaking change, but renovate landed it on repos that already had tsconfig.json files so it went unnoticed.

@bradenmacdonald
Copy link
Contributor

Looking at the log, it shows errors about @edx/frontend-component-header/dist/studio-header/StudioHeader.tsx which seems wrong for two reasons: (1) this is not part of the authoring MFE, so why is it referencing the Studio header at all?, and (2) why are there .tsx files in the dist folder ? Seems like frontend-component-header was not built properly. (Or are we doing some kind of hybrid build that outputs .tsx files?!)

@brian-smith-tcril
Copy link
Contributor

brian-smith-tcril commented Sep 24, 2025

Looking at the log, it shows errors about @edx/frontend-component-header/dist/studio-header/StudioHeader.tsx which seems wrong for two reasons: (1) this is not part of the authoring MFE, so why is it referencing the Studio header at all?, and (2) why are there .tsx files in the dist folder ? Seems like frontend-component-header was not built properly. (Or are we doing some kind of hybrid build that outputs .tsx files?!)

The build step does seem a bit odd (https://github.com/openedx/frontend-component-header/blob/ecf7f1dfc1da11ae32c36f4cae5fcfb45733ea5b/Makefile#L8-L14)

For now I've just marked the 6.5.x and 6.6.x versions as deprecated on NPM and released the latest version as 7.0.0

feanil and others added 2 commits September 25, 2025 10:13
Remove @edx/reactifex package from devDependencies as it is no longer
needed. Translation extraction functionality has been verified to work
correctly without this dependency.

Co-Authored-By: Claude <noreply@anthropic.com>
This test should be suppling a `courseModes` parameter not a
`courseMode` parameter with a missing `s`.
@feanil feanil force-pushed the feanil/remove-reactifex-packages branch from 41d8ef0 to 1c9e20e Compare September 25, 2025 14:14
@feanil feanil marked this pull request as ready for review September 25, 2025 14:17
@feanil feanil enabled auto-merge September 25, 2025 14:52
@feanil feanil merged commit 59dbee3 into master Sep 25, 2025
7 checks passed
@feanil feanil deleted the feanil/remove-reactifex-packages branch September 25, 2025 17:15
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