types: Migrated todo_widget.js to todo_widget.ts.

[Wr4th100]

• Migrated todo_widget.js to Typescript for improved type safety and maintainability.
• Renamed the file in tests-js-with-node.

Self-review checklist

• Self-reviewed the changes for clarity and maintainability
  (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

• Explains differences from previous plans (e.g., issue description).
• Highlights technical choices and bugs encountered.
• Calls out remaining decisions and concerns.
• Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

• Each commit is a coherent idea.
• Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

• Visual appearance of the changes.
• Responsiveness and internationalization.
• Strings and tooltips.
• End-to-end functionality of buttons, interactions and flows.
• Corner cases, error conditions, and easily imagined bugs.

[timabbott]

This feels very hard to review; git show doesn't even think it's the same file. It looks like you've got a lot of new logic to condition on undefined situations that are not real scenarios; like this:

+            const taskValue = $elem?.find("input.add-task")?.val();
+            const task = (typeof taskValue === "string" ? taskValue : "").trim();
+            const descValue = $elem?.find("input.add-desc")?.val();
+            const desc = (typeof descValue === "string" ? descValue : "").trim();

These should largely be done via adding assert statements or !s, not creating new impossible scenarios involving using "" as a value if something we think impossible happens.

You also seem to have re-sorted the functions. Please don't do that in a TS migration commit; if you need to do it, do it in an easily reviewed function either before or after.

@Wr4th100 can you rework this to be more reviewable?

[Wr4th100]

You also seem to have re-sorted the functions. Please don't do that in a TS migration commit; if you need to do it, do it in an easily reviewed function either before or after.

I changed the order, because eslint was complaining about it (@typescript-eslint/member-ordering). I have disabled the eslint check now. git now recognizes it as the same file.

These should largely be done via adding assert statements or !s, not creating new impossible scenarios involving using "" as a value if something we think impossible happens.

Sure, I have made the changes now, please have a look!

[Wr4th100]

@timabbott, I have made the necessary changes, and all the checks have passed, please have a look!

[LaPulgaaa]

@Wr4th100 Can you provide an update on this migration? This surely requires a rebase if anything. It would be nice to get this migrated!

[Wr4th100]

Normally, the inbound function receives the sender_id argument as number. But here, they have mentioned canned here. I have changed that to message_sender_id.

Is this correct? Or should I be changing something else?

[afeefuddin]

Change the type annotations of the parameter of the inbound function to accept string | number, otherwise, it might result in a key error. You can read the CZO thread for more context.

[afeefuddin]

Also, when making these types of changes, it's recommended to create a separate commit and provide a valid reason for why you chose that approach.

[afeefuddin]

Hey @Wr4th100, are you looking forward to completing this one?

[timabbott]

@Wr4th100 can you comment on how you resolved the feedback? I guess @N-Shar-ma should probably review this in any case.