Skip to content

Delete TODOs we're never going to fix#3197

Merged
benchristel merged 7 commits intomainfrom
benc/delete-old-todos
Feb 3, 2026
Merged

Delete TODOs we're never going to fix#3197
benchristel merged 7 commits intomainfrom
benc/delete-old-todos

Conversation

@benchristel
Copy link
Member

@benchristel benchristel commented Jan 22, 2026

Some of these TODOs represent the wishful thinking of our forebears a decade
ago. Others have become obsolete as code changed around them. I'd like to be
able to grep for TODO in our codebase and get a sense of where the real tech
debt is, and having old, won't-fix TODOs makes that difficult.

If any of the removed TODOs are truly important, I trust they'll come back.

Issue: none

Test plan:

none

@github-actions
Copy link
Contributor

github-actions bot commented Jan 22, 2026

🗄️ Schema Change: No Changes ✅

@github-actions
Copy link
Contributor

github-actions bot commented Jan 22, 2026

🛠️ Item Splitting: No Changes ✅

@github-actions
Copy link
Contributor

github-actions bot commented Jan 22, 2026

Size Change: 0 B

Total Size: 485 kB

ℹ️ View Unchanged
Filename Size
packages/kas/dist/es/index.js 20.8 kB
packages/keypad-context/dist/es/index.js 1 kB
packages/kmath/dist/es/index.js 5.98 kB
packages/math-input/dist/es/index.js 98.5 kB
packages/math-input/dist/es/strings.js 1.61 kB
packages/perseus-core/dist/es/index.item-splitting.js 11.8 kB
packages/perseus-core/dist/es/index.js 24.8 kB
packages/perseus-editor/dist/es/index.js 99.2 kB
packages/perseus-linter/dist/es/index.js 8.83 kB
packages/perseus-score/dist/es/index.js 9.32 kB
packages/perseus-utils/dist/es/index.js 403 B
packages/perseus/dist/es/index.js 187 kB
packages/perseus/dist/es/strings.js 7.44 kB
packages/pure-markdown/dist/es/index.js 1.39 kB
packages/simple-markdown/dist/es/index.js 6.71 kB

compressed-size-action

@github-actions
Copy link
Contributor

github-actions bot commented Jan 22, 2026

npm Snapshot: Published

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

Example:

pnpm add @khanacademy/perseus@PR3197

If you are working in Khan Academy's frontend, you can run the below command.

./dev/tools/bump_perseus_version.ts -t PR3197

If you are working in Khan Academy's webapp, you can run the below command.

./dev/tools/bump_perseus_version.js -t PR3197

@benchristel benchristel force-pushed the benc/delete-old-todos branch from cfcd6de to 2d6edf0 Compare February 2, 2026 18:25
@benchristel benchristel marked this pull request as ready for review February 2, 2026 18:25
/**
* @deprecated - do not use in new code.
*/
getSerializedState(): State;
Copy link
Member Author

Choose a reason for hiding this comment

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

getSerializedState is deprecated; hence, we're never going to type this.

showWordCount={true}
warnNoPrompt={false}
warnNoWidgets={true}
// TODO(LEMS-2656): remove TS suppression
Copy link
Member Author

Choose a reason for hiding this comment

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

There is no TS suppression here.

Comment on lines -162 to -163
// TODO(eater): The 22nd function added will be {(x) since '{'
// comes after 'z'
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a funny bug, but the interaction widget is very dead, so we are never going to fix it.

/**
* An answer item in the choices list.
*
* TODO(michaelpolyak): Implement answer reordering, CP-117
Copy link
Member Author

Choose a reason for hiding this comment

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

CP-117 has been marked Won't Do.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, I think our label-image widget editor supports re-ordering by dragging.

// it can be triggered from within the widget, or requires
// a similar implementation locally, in which case consider
// refactoring `perseus-admin-package/image-upload-dialog.jsx`
// as common utility, CP-118
Copy link
Member Author

Choose a reason for hiding this comment

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

CP-118 has been marked Won't Do.

Copy link
Member Author

Choose a reason for hiding this comment

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

Molecule is a dead widget.

Comment on lines -1171 to 1174
// TODO(kevinb) actually compute the size of the graphie correctly and
// make it that size so we don't have to add extra padding. The value
// was determined by eye-balling the layout. :(
const paddingForBottomLabel = 75;
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a gross hack, but plotter is deprecated and hidden from the widget dropdown in the editors, so it's probably not worth fixing.

// be independent of everything else.
import type {KeypadKey} from "./keypad";

// TODO(FEI-4010): Remove `Perseus` prefix for all types here
Copy link
Collaborator

Choose a reason for hiding this comment

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

commentary Hm. This ticket was closed as Won't Do, but I think it's still a valuable thing. React doesn't prefix its symbols with ReactXyz (typically) and we could shorten alot of our types by removing the prefix. That said, probably a very low-value set of work.

TLDR: I'm good with removing this TODO. :)

{value: "", content: <span>&mdash;</span>},
{value: "->", content: <span>&#x2192;</span>},
/*
TODO(eater): fix khan-exercises so these are supported
Copy link
Collaborator

Choose a reason for hiding this comment

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

khan-exercises?? 😂 That's a while ago

/**
* An answer item in the choices list.
*
* TODO(michaelpolyak): Implement answer reordering, CP-117
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, I think our label-image widget editor supports re-ordering by dragging.


/* TODO(benkomalo): ughhh. kill or move this out of here :(
Styles specifically for the mobile native apps */
/* Styles specifically for the mobile native apps */
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder ... if this is used now that mobile uses the web experience. 🤔 No change necessary on this PR, but can this be removed entirelY?

@benchristel benchristel changed the title Delete TODOs we're never going to fix Delete TODOs we're never going to fix - Feb 2, 2026
@benchristel benchristel changed the title Delete TODOs we're never going to fix - Delete TODOs we're never going to fix Feb 2, 2026
@benchristel benchristel merged commit 7fcef1a into main Feb 3, 2026
11 of 25 checks passed
@benchristel benchristel deleted the benc/delete-old-todos branch February 3, 2026 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants