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

Remove unused APIOptions and types #1301

Merged
merged 5 commits into from
May 23, 2024
Merged

Remove unused APIOptions and types #1301

merged 5 commits into from
May 23, 2024

Conversation

jeremywiebe
Copy link
Collaborator

@jeremywiebe jeremywiebe commented May 22, 2024

Summary:

This PR removes some code/APIOptions that are completely unused today. To verify, I checked the webapp and mobile source code. I also checked and these options are not used in Perseus so even if they were set by calling applications, they do nothing.

It's time to go!

  • Remove Empty type as its now centrally defined in utility.d.ts which is symlinked into each package src
  • Remove unused and deprecated inModal APIOption
  • Remove unused and deprecated useDraftEditor APIOption

Issue: "none"

Test plan:

@jeremywiebe jeremywiebe self-assigned this May 22, 2024
Copy link
Contributor

github-actions bot commented May 22, 2024

Size Change: -169 B (-0.02%)

Total Size: 839 kB

Filename Size Change
packages/perseus/dist/es/index.js 403 kB -169 B (-0.04%)
ℹ️ 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.5 kB
packages/math-input/dist/es/strings.js 1.73 kB
packages/perseus-core/dist/es/index.js 908 B
packages/perseus-editor/dist/es/index.js 269 kB
packages/perseus-error/dist/es/index.js 877 B
packages/perseus-linter/dist/es/index.js 21.8 kB
packages/perseus/dist/es/strings.js 3.22 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 May 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.66%. Comparing base (df44ef7) to head (46e7b84).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1301      +/-   ##
==========================================
+ Coverage   68.76%   70.66%   +1.89%     
==========================================
  Files         477      481       +4     
  Lines      101981   102034      +53     
  Branches     7250    10987    +3737     
==========================================
+ Hits        70130    72100    +1970     
+ Misses      31673    29934    -1739     
+ Partials      178        0     -178     

Impacted file tree graph

Files Coverage Δ
packages/perseus/src/perseus-api.tsx 100.00% <ø> (ø)
packages/perseus/src/widgets/definition.tsx 90.24% <ø> (-0.24%) ⬇️
packages/perseus/src/widgets/explanation.tsx 92.34% <ø> (+0.32%) ⬆️

... and 144 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 df44ef7...46e7b84. Read the comment docs.

@jeremywiebe jeremywiebe marked this pull request as ready for review May 22, 2024 17:36
@jeremywiebe jeremywiebe requested review from handeyeco and a team May 22, 2024 17:36
@khan-actions-bot
Copy link
Contributor

Gerald

Required Reviewers
  • @Khan/perseus for changes to .changeset/brave-lions-melt.md, .changeset/wild-cobras-applaud.md, packages/perseus/src/perseus-api.tsx, packages/perseus/src/perseus-types.ts, packages/perseus/src/types.ts, packages/perseus/src/widgets/definition.tsx, packages/perseus/src/widgets/explanation.tsx, packages/perseus-editor/src/widgets/__stories__/expression-editor.stories.tsx, packages/perseus-editor/src/widgets/__stories__/label-image-editor.stories.tsx, packages/perseus-editor/src/widgets/__stories__/radio-editor.stories.tsx

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

Copy link
Contributor

npm Snapshot: Published

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

Example:

yarn add @khanacademy/perseus@PR1301

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

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

Copy link
Member

@benchristel benchristel left a comment

Choose a reason for hiding this comment

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

Yay! Thanks for cleaning these up.

@jeremywiebe
Copy link
Collaborator Author

For posterity, I already landed and deployed a webapp change that removes usage of these options. They are optional and so webapp is fully ready for these changes in this PR whenever they are released.

@jeremywiebe jeremywiebe merged commit 1ca5a12 into main May 23, 2024
22 checks passed
@jeremywiebe jeremywiebe deleted the jer/remove-unused branch May 23, 2024 22:36
benchristel pushed a commit that referenced this pull request May 28, 2024
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated.


# Releases
## @khanacademy/perseus@23.0.0

### Major Changes

-   [#1301](#1301) [`1ca5a12aa`](1ca5a12) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Remove unused/deprecated APIOptions: useDraftEditor and inModal

### Minor Changes

-   [#1283](#1283) [`3b85777c7`](3b85777) Thanks [@daniellewhyte](https://github.com/daniellewhyte)! - Make horizontal divider invisible to screen reader


-   [#1305](#1305) [`ec600a11e`](ec600a1) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Creation of the new Mafs-based Quadratic Graph for the Interactive Graph Widget


-   [#1310](#1310) [`e6fc912bf`](e6fc912) Thanks [@benchristel](https://github.com/benchristel)! - Upgrade to Mafs 0.18.7, which lets us draw graph axes with thicker lines.

### Patch Changes

-   [#1312](#1312) [`925f4ee03`](925f4ee) Thanks [@benchristel](https://github.com/benchristel)! - Fix Safari-specific bug where axis tick numbers did not appear


-   [#1301](#1301) [`1ca5a12aa`](1ca5a12) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Remove duplicate `Empty` type

-   Updated dependencies \[[`3b85777c7`](3b85777)]:
    -   @khanacademy/simple-markdown@0.12.0
    -   @khanacademy/pure-markdown@0.3.5

## @khanacademy/perseus-editor@6.6.0

### Minor Changes

-   [#1298](#1298) [`b84e4a981`](b84e4a9) Thanks [@nishasy](https://github.com/nishasy)! - [Interactive Graph Editor] Setting a locked line's length to 0 (equal points) will stop the exercise from saving via getSaveWarnings()


-   [#1305](#1305) [`ec600a11e`](ec600a1) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Creation of the new Mafs-based Quadratic Graph for the Interactive Graph Widget

### Patch Changes

-   [#1301](#1301) [`1ca5a12aa`](1ca5a12) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Remove duplicate `Empty` type

-   Updated dependencies \[[`1ca5a12aa`](1ca5a12), [`3b85777c7`](3b85777), [`ec600a11e`](ec600a1), [`925f4ee03`](925f4ee), [`1ca5a12aa`](1ca5a12), [`e6fc912bf`](e6fc912)]:
    -   @khanacademy/perseus@23.0.0

## @khanacademy/simple-markdown@0.12.0

### Minor Changes

-   [#1283](#1283) [`3b85777c7`](3b85777) Thanks [@daniellewhyte](https://github.com/daniellewhyte)! - Make horizontal divider invisible to screen reader

## @khanacademy/pure-markdown@0.3.5

### Patch Changes

-   Updated dependencies \[[`3b85777c7`](3b85777)]:
    -   @khanacademy/simple-markdown@0.12.0

## @khanacademy/perseus-dev-ui@1.5.9

### Patch Changes

-   Updated dependencies \[[`3b85777c7`](3b85777)]:
    -   @khanacademy/simple-markdown@0.12.0
    -   @khanacademy/pure-markdown@0.3.5

Author: khan-actions-bot

Reviewers: benchristel

Required Reviewers:

Approved By: benchristel

Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ⏭️  Publish npm snapshot, ✅ Cypress (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1308
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