Skip to content

Comments

Update reader conversations tests to typeScript#101915

Merged
Addison-Stavlo merged 7 commits intotrunkfrom
update/reader-actions-to-typescript
Mar 28, 2025
Merged

Update reader conversations tests to typeScript#101915
Addison-Stavlo merged 7 commits intotrunkfrom
update/reader-actions-to-typescript

Conversation

@Addison-Stavlo
Copy link
Contributor

Related to # #101792 - in reviewing this @mehmoodak noted the opportunity to convert a couple of these files to typescript.

Proposed Changes

Updates a couple reader files to typescript.

Why are these changes being made?

  • Ongoing effort to convert files to typescript to improve devex.

Testing Instructions

  • smoke test
  • ensure checks pass

Pre-merge Checklist

  • Has the general commit checklist been followed? (PCYsg-hS-p2)
  • Have you written new tests for your changes?
  • Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • Have you checked for TypeScript, React or other console errors?
  • Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
  • Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
    • For UI changes, have we tested the change in various languages (for example, ES, PT, FR, or DE)? The length of text and words vary significantly between languages.
  • For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

@Addison-Stavlo Addison-Stavlo requested a review from a team as a code owner March 26, 2025 18:21
@matticbot matticbot added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Mar 26, 2025
@github-actions
Copy link

github-actions bot commented Mar 26, 2025

@matticbot
Copy link
Contributor

matticbot commented Mar 26, 2025

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@matticbot
Copy link
Contributor

matticbot commented Mar 26, 2025

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

  • blaze-dashboard
  • notifications
  • wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug update/reader-actions-to-typescript on your sandbox.

Copy link
Member

@mehmoodak mehmoodak left a comment

Choose a reason for hiding this comment

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

Thanks for refactoring the code to TypeScript.

} from 'calypso/state/reader/conversations/actions';
import { CONVERSATION_FOLLOW_STATUS } from 'calypso/state/reader/conversations/follow-status';

type Dispatch = jest.Mock< any, any >;
Copy link
Member

Choose a reason for hiding this comment

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

Can we use this from

import { Dispatch } from 'redux';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That wouldn't be a mock function would it? That would just be the standard redux dispatch?

Copy link
Member

Choose a reason for hiding this comment

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

You're right. VSCode doesn't give any error to me when I used it, maybe it was waiting on Intellisense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I feel like the more we use TS here the slower intellisense becomes. 😢

Copy link
Member

Choose a reason for hiding this comment

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

Looking forward to TS go-compiler with 10x improvement 🤞

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be awesome.

import { CONVERSATION_FOLLOW_STATUS } from 'calypso/state/reader/conversations/follow-status';

type Dispatch = jest.Mock< any, any >;
type GetState = () => Record< string, unknown >;
Copy link
Member

Choose a reason for hiding this comment

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

Better if we can add an interface for the state. Not a blocker.

@Addison-Stavlo
Copy link
Contributor Author

This was WSOD'ing the reader due to the init file not properly initializing the reader store after being converted to typescript. I tried a couple things that I thought would resolve the issue but I keep running into errors.

Im going to leave init.js the way it is. It doesn't seem very beneficial to change to typescript anyways.

@Addison-Stavlo Addison-Stavlo merged commit 9c4c2fc into trunk Mar 28, 2025
13 checks passed
@Addison-Stavlo Addison-Stavlo deleted the update/reader-actions-to-typescript branch March 28, 2025 14:19
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 28, 2025
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