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: replace structuredClone with _cloneDeep #1243

Merged
merged 2 commits into from
Jan 6, 2025
Merged

Conversation

brobro10000
Copy link
Member

@brobro10000 brobro10000 commented Jan 6, 2025

Replaces instances of structuredClone with _cloneDeep. This is done to increase overall access to the learner portal for browsers that do not support structuredClone

This PR also updates the @edx/browserslist-config to the latest version 1.4.0 (npm).

For all changes

  • Ensure adequate tests are in place (or reviewed existing tests cover changes)
  • Ensure English strings are marked for translation. See documentation for more details.

Only if submitting a visual change

  • Ensure to attach screenshots
  • Ensure to have UX team confirm screenshots

Copy link

codecov bot commented Jan 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.00%. Comparing base (619db53) to head (dabc542).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1243      +/-   ##
==========================================
- Coverage   89.03%   89.00%   -0.04%     
==========================================
  Files         403      403              
  Lines        8729     8729              
  Branches     2083     2126      +43     
==========================================
- Hits         7772     7769       -3     
- Misses        916      918       +2     
- Partials       41       42       +1     

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

Copy link
Member

@adamstankiewicz adamstankiewicz left a comment

Choose a reason for hiding this comment

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

LGTM. Just a heads up, though, we probably shouldn't optimize too much for older browsers that we don't officially support. For example, looking at one of the error instances in Datadog shows a user using Chrome v87, when Open edX technically intends to support the latest 2 major versions; The edX.org support center article about browser versions denotes versions >100 of Chrome.

That said, given we already use _cloneDeep in at least one other spot, makes sense to utilize it as we're not installing any additional NPM packages in this change.

Related to browser support, you might also consider upgrading @edx/browserslist-config from pinned 1.1.1 to the latest version 1.4.0, which adopts the recommended "best practices" around browserslist usage. See https://github.com/openedx/browserslist-config/blob/42abbdb624ce64667a3246515d5cbc387dddc35f/index.js#L14-L20 for more details.

tl;dr; @edx/browserslist-config now also supports "last 2 versions and not dead" + "> 0.2% and not dead".

Here's a comparison of supported browser versions between these 2 releases of @edx/browserslist-config:

v1.1.1

and_chr 131
and_ff 132
chrome 131
chrome 130
edge 131
edge 130
firefox 133
firefox 132
ios_saf 18.2
ios_saf 18.1
ios_saf 18.0
ios_saf 17.6-17.7
ios_saf 17.5
ios_saf 17.4
ios_saf 17.3
ios_saf 17.2
ios_saf 17.1
ios_saf 17.0
ios_saf 16.6-16.7
ios_saf 16.5
ios_saf 16.4
ios_saf 16.3
ios_saf 16.2
ios_saf 16.1
ios_saf 16.0
safari 18.2
safari 18.1
safari 18.0
safari 17.6
safari 17.5
safari 17.4
safari 17.3
safari 17.2
safari 17.1
safari 17.0

v1.4.0

and_chr 131
and_ff 132
and_qq 14.9
and_uc 15.5
android 131
chrome 131
chrome 130
chrome 129
chrome 128
chrome 127
chrome 126
chrome 125
chrome 124
chrome 109
edge 131
edge 130
edge 129
edge 128
firefox 132
firefox 131
firefox 130
firefox 129
firefox 115
ios_saf 18.1
ios_saf 18.0
ios_saf 17.6-17.7
ios_saf 17.5
ios_saf 17.4
ios_saf 17.3
ios_saf 17.2
ios_saf 17.1
ios_saf 17.0
ios_saf 16.6-16.7
ios_saf 16.5
ios_saf 16.4
ios_saf 16.3
ios_saf 16.2
ios_saf 16.1
ios_saf 16.0
ios_saf 15.6-15.8
kaios 3.0-3.1
kaios 2.5
op_mini all
op_mob 80
opera 114
opera 113
opera 112
safari 18.1
safari 18.0
safari 17.6
safari 17.5
safari 17.4
safari 17.3
safari 17.2
safari 17.1
safari 17.0
safari 16.6
safari 15.6
samsung 26
samsung 25

src/components/app/data/services/programs.js Outdated Show resolved Hide resolved
@brobro10000 brobro10000 merged commit 4c04a62 into master Jan 6, 2025
5 of 6 checks passed
@brobro10000 brobro10000 deleted the hu/ent-9881 branch January 6, 2025 17:44
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