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

Clarify what versions of React the project supports #1039

Closed
wants to merge 2 commits into from

Conversation

jeremywiebe
Copy link
Collaborator

@jeremywiebe jeremywiebe commented Feb 28, 2024

Summary:

We received a PR that opened up the versions of React for simple-markdown a bit. This was a good idea so I've created a PR to clarify what versions of React we support.

When we complete the React v18 migration, we'll need to update these, but this puts the ranges in place so we're prepared.

Issue: "none"

Test plan:

@jeremywiebe jeremywiebe self-assigned this Feb 28, 2024
@jeremywiebe jeremywiebe marked this pull request as ready for review February 28, 2024 17:02
@khan-actions-bot khan-actions-bot requested a review from a team February 28, 2024 17:02
@khan-actions-bot
Copy link
Contributor

khan-actions-bot commented Feb 28, 2024

Gerald

Required Reviewers
  • @Khan/perseus for changes to yarn.lock, .changeset/plenty-forks-sing.md, packages/math-input/package.json, packages/perseus/package.json, packages/perseus-editor/package.json, packages/simple-markdown/package.json

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

Copy link
Contributor

github-actions bot commented Feb 28, 2024

Size Change: +2.11 kB (0%)

Total Size: 821 kB

Filename Size Change
packages/perseus-editor/dist/es/index.js 266 kB -82 B (0%)
packages/perseus/dist/es/index.js 393 kB +2.19 kB (+1%)
ℹ️ View Unchanged
Filename Size
packages/kas/dist/es/index.js 38.1 kB
packages/kmath/dist/es/index.js 4.27 kB
packages/math-input/dist/es/index.js 80 kB
packages/perseus-core/dist/es/index.js 908 B
packages/perseus-error/dist/es/index.js 878 B
packages/perseus-linter/dist/es/index.js 21.8 kB
packages/pure-markdown/dist/es/index.js 3.68 kB
packages/simple-markdown/dist/es/index.js 12.4 kB

compressed-size-action

Copy link

codecov bot commented Feb 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.38%. Comparing base (82f6a63) to head (b5a8e2c).
Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1039      +/-   ##
==========================================
+ Coverage   64.35%   66.38%   +2.03%     
==========================================
  Files         427      436       +9     
  Lines       97258    97766     +508     
  Branches     6451    10128    +3677     
==========================================
+ Hits        62586    64906    +2320     
+ Misses      34672    32860    -1812     

Impacted file tree graph

see 114 files with indirect coverage changes


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 82f6a63...b5a8e2c. Read the comment docs.

Copy link
Contributor

npm Snapshot: Published

Good news!! We've packaged up the latest commit from this PR (b5a8e2c) and published it to npm. You
can install it using the tag PR1039.

Example:

yarn add @khanacademy/perseus@PR1039

If you are working in Khan Academy's webapp, you can run:

./dev/tools/bump_perseus_version.sh -t PR1039

Copy link
Contributor

@Myranae Myranae 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! How do you know what the minimum version for these packages is though? Do these need to be tested?

@jeremywiebe
Copy link
Collaborator Author

Looks good! How do you know what the minimum version for these packages is though? Do these need to be tested?

The min version Perseus supports is mostly defined by what APIs we use. In reality, our minimum should be at what webapp and mobile are using. FEI is working on moving everything to React 18. I have #1064 open that moves Perseus to React 18, so I'm inclined to abort this PR as its changes would be very short-lived.

@jeremywiebe
Copy link
Collaborator Author

Closing as #1064 will obviate the need for this PR once it lands (shortly!)

@jeremywiebe jeremywiebe closed this Jul 4, 2024
@jeremywiebe jeremywiebe deleted the react-deps branch July 4, 2024 21:55
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