-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Purchases: Convert me/purchases/utils to TS #97297
Conversation
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
This PR modifies the release build for the following Calypso Apps: For info about this notification, see here: PCYsg-OT6-p2
To test WordPress.com changes, run |
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! All test's have passed. Bill and I had a couple of learning questions, but no blockers
client/me/purchases/utils.ts
Outdated
return ! props.hasLoadedSites || ! props.hasLoadedUserPurchasesFromServer; | ||
} | ||
|
||
function canEditPaymentDetails( purchase ) { | ||
export function canEditPaymentDetails( purchase: Purchase ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like this would expect a boolean as a response. Should it be included as the return type here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A good question! It does have an implicit boolean return value, but TypeScript can infer this and got lazy 😅 . Unlike PHP, inferred types are just as valid and strict as explicit types, so if if you tried to, say, use string manipulation on the result of this function, you wouldn't be able to compile it (eg: "cannot use this boolean as a string").
The advantage of adding an explicit return type would be that if this function currently or in the future tried to return something different, a type error would appear here (eg: "this function returns a boolean but it can return a string"). By leaving the return type off, we're telling TypeScript, "the return type of this function is whatever it can return". If we modified it to sometimes return a string, the implicit return type would become boolean|string
and TypeScript would make sure that everywhere that called it was ok with having a string sometimes.
It's a trade-off either way. This SO answer does a pretty good job of explaining it. I generally prefer adding return types but when converting existing code sometimes I don't as an expedient. That said, I may go back and add them here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's an example of the inferred types at work. When converting this function to TS maybe I don't know what it's supposed to return, so I can just let TS figure it out:
That's usually the reason to leave off a return type: it's not clear or maybe it's something really complicated, in which case TS can handle it.
client/me/purchases/utils.ts
Outdated
return addNewPaymentMethod; | ||
} | ||
|
||
function isTemporarySitePurchase( purchase ) { | ||
export function isTemporarySitePurchase( purchase: Purchase ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be returning a string? If so, should be typed as such? When is it best to type the return and when not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully I was able to explain this somewhat above. TLDR when converting existing functions, IMO it's ok to leave off return types. It's way more important to have a file be using TypeScript than for it to have perfect types.
* Convert me/purchases/utils to TS * Prevent calling purchase functions without purchase * Export utils functions directly * Add explicit return types to all util functions
* Return showing duration days back Related to the commit 3fcd217. * Update Playwright to 1.48.2 (#97275) We've been running an old Playwright, which uses Chromium 116 (Aug 2023). Not a great thing for e2e tests. Trying to bump to a recent one (1.49.1 fails, so I'm guessing a regression). 1.48.2 is Chromium 130 (Oct 2024), so should work and all pass. We need to manually install browsers now tho: https://github.com/microsoft/playwright/releases/tag/v1.38.0 * e2e tests: Firefox for Woo tests (#97335) * E2E Tests: Fix LOHP tests (#97338) Scrolling list of themes needs to be hovered on before click, otherwise not considered stable by Playwright. * Plans Step: TS and function form (#96786) Co-authored-by: Omar Alshaker <omar@omaralshaker.com> * E2E Tests: Lower workers from 100% to 50% (#97334) This means out of 16 cores, jest will use 8. Might be better, might be worse, but worth a try if CPU is capped at 100%. * Purchases: Make Refund Window Clearer (#89249) * Update context-links.js * Include refund information * Update strings * Address review * Fix string * Address review * Address review * Address review * Remove support link * Simply switch to "Refund available" * Fix style * Update style.scss * Use the default version of Odie specified in the backend (#97333) * Help Center Experience: ramp to 75% of users (#97271) * Performance Profiler: Pass locale to LLM Query (#97285) * Performance Profiler: Pass locale to LLM Query * Add locale slug to the LLM react query key * Use the builtin _locale query param when using wpcom requests It is not necessary to use an explicit locale parameter * Launches the Application Password flow (#97345) * Site Migration: Update Plans page for migrations flow (#97062) * Initial start at new plans page for migrations * Update loader styles/markup * Fix header styles * Remove descriptions * Fix spacing * Refactor pricing * Fix typing * Fix checkout ctas, update wording for 2 year plan * Simplify loader * Simplify pricing output * Tweak copy, remove unused function param * Simplify rendering * Add vars for repeated constants * Revert changes to Skeleton * Revert unnecessary name change * Fix tabs * Fix for loader display * Simplify * Prevent flash of uncentered loader * Update language for consistency * More specific typing, pass planSlugs through so we don't have to repeat * Use feature flag being introduced in separate PR * Use consistent variable * Ensure we have planSlugs before reducer * Update test * Fix mobile spacing, update pricing copy * Mobile refund text * Fix flash of original skeleton * Fix box shadow on secondary buttons * Don't show storage * Use styles to make first list item bold, tweaks to spacing * No bold text for feature flag * Add refund tooltip * Account for border width * Update spacing under header * Fix title spacing * Bold around days * New strings for features * Separate Jetpack and WPcom features * Fix jetpack logo color * Logo spacing * Move so we can take full advantage of hooks in translate, add bold text * Revert changes to plan feature list * Remove unused prop * Fix button colors * Alignment issues * Fix alignment issues * Update loader colors to gray * Calculate percentage rather than hard-coding, consolidate features * Stats: Gate currently open summary pages (#97274) * Add missing modules to gated stats * Add constant for file downloads * Add new upsell copy * Reorder switch cases * Update upsell copy * Maintain current behaviour for gated modules * Add events for opening interval dropdowns (#97309) * Add events for opening interval dropdowns * Remove useMemo for hasWPCOMPlanInCart * Notifications: Restore viewbox for Reblog icon (#97357) Fixes the display of Reblog gridicons in Notifications. Introduced in #79932. * Holiday snow: Pass siteIsWpcom prop (#97363) This prop was not being passed when the setting is used on tangled calypso and so the holiday snow setting panel was not being displayed. * Add support for developer.wordpress.com in localizeUrl (#96655) * Checkout: Make PayPal PPCP available to everyone if server allows it (#97350) * Remove checkout/paypal-ppcp config option * Remove "a8c-only" from PayPal PPCP button * Do not show both paypal-express and paypal-js in checkout * Only mark PPCP enabled if enabled, not just existing * A4A: fix overdue invoice banner typo (#97303) * A4A > Feedback: Handle save feedback (#97106) * A4A: automated feedbacks UI * show feedback after some actions are performed * logic changes * show notification only if feedback is not shown * logic update * initial API integration * wire up to existing endpoint * show success notice when feedback is successfully sent * lint fix * update success notice copy based on demo video in p2 --------- Co-authored-by: Andrii <lysenkoa.work@gmail.com> Co-authored-by: Travis Walter <travis@automattic.com> * Paid stats: update upsell copy in Stats modules (#97366) * Update upsell copy in Stats modules * Update upsell copy in Stats module including a link to support documentation. * Update upsell copy for file downloads * Update StatsCardUpsell to specify the upsell plan for WPCOM * Update buttonLabel typing - adding TranslateResult type * Enable paid stats v3 upsell in production config (#97310) * Update copy on old modal for personal/premium upsell features (#97312) * Fix WCCOM account creation flow UI (#97311) * Fix flash placeholder in lost password screen * Fix flash blue background * Refactor UserStep component to use isWoo prop for WooCommerce-specific logic and skip account completion logic for Woo * Add comment * Update client/layout/masterbar/woo.scss Co-authored-by: Ilyas Foo <foo.ilyas@gmail.com> --------- Co-authored-by: Ilyas Foo <foo.ilyas@gmail.com> * Update masterbar and jetpack header logo (#97318) * Update masterbar and jetpack header logo * Fix transform for old logo * Update Woo branding for reset password pages (#97152) * Add changes for reset password and fixes so rebrand styling update do not affect core profiler flow * Use oauth class instead for more precise selector * Update test * Hosting dashboard: prevent showing invalid intermediate content when switching from Atomic to Simple (#97268) * Design Picker: Dedupe the themes (#97315) * Design Picker: Dedupe the themes * Address feedback * Fix the order * Design Picker: Display the recommended themes (#97320) * Design Picker: Add the Recommended themes section * Design Picker: Don't show recommended themes if the selected categories have been changed * Fix types * Show no results only when the recommended designs section is hidden * Fix track property * Paid stats: update UTM and Devices on empty state to point to the right support URL (#97376) * Update UTM module to conditionally render the support link * Update Devices module to conditionally render the support link * Update condition to use isJetpackSite selector * A4A > Feedback: Enable in-product feedback mechanism on staging (#97388) * Domain management: Match component design in the case where user has Mailboxes (#97252) * Extend EmailComparison component to support adding container className * Customize EmailComparison component to match figma design * Customize EmailComparison component to match figma design * Replace feature flag with usage context * Rename constant to be a more accurate name * Extend EmailPlanMailboxesList props to customize component * Extend EmailPlan props to customize component * Customize EmailPlan component based on feature flag * Add context-all-domain-management style * Show optional icon next to the mailbox title based on type * Replace feature flag with context property * Add mailbox icon based on context * Rename constant to be a more accurate name * Simplify header icon rendering * Use destructor inside a function param * Hide MailPoet upsell for all domains management context * Add missing align-items property style * Update children type definition * Adjust section header text and button font size * Update color variable * Rearange css selector to avoid important operator * Remove custom logic for getting email icon * Fix alignment issue * Design Picker: Add free tier filter to Type category in design picker (#97323) * add free filter to type filters * remove free only filter component * Just move free-only toggle into types and keep functionalites the same * Update track events --------- Co-authored-by: arthur <arthur.chu@automattic.com> * A4A > Feedback: Minor typo fix (#97397) * Login: Fix Safari autozoom (#97374) * Fix coupons in Stepper (#97359) * Revert "Revert "Help Center: Get unread count when new message arrives (#97120)"" (#97402) * E2E Tests: Disable QUIC as it's sometimes unstable (#97403) and browsers don't fallback :( * [Odie] We've missed to add the styling for error messages in the new help center experience (#97405) * Duplicate views: Redirect from Calypso to WP Admin (Posts, Testimonials, Portfolio) (#97231) * E2E: Fix the LOHP theme test (#97346) Some back-and-forth to try and unflaky this test... Mix of race condition abort/goto and not being able to hover->click. --------- Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com> * Accessiblity: Fix Help Center portal injection (#97370) Co-authored-by: Kosta <heavyweight@users.noreply.github.com> * Fix Stepper performance bug (#97354) * Stepper: Fix 404 error (#97407) * Conditionally update copy on migrate vs. import text page (#97125) * Change Calypso Logo generator prompts (#97171) * accept both remote and base64 urls and produce a well formed request for each * move first logo generation prompt to backend * use new flux request if a style is used as URL hash * add image types and constants, add featuresControl to state * remove style setting from URL hash * modify hook functions to accept style for the request * implement styles dropdown on logo generator prompt * wee css fixes: spacing actions and make button logo (SVG) same color as button * Keep accent color untouched in Stepper (#97371) * Reader: Added tracks event for time spent reading the post. (#97389) * Reader: Added tracks event for time spent reading the post. * Track reading time when the current post changes. * Help Center: fire event when requesting support (#97419) * Use selectedSite from storage * Fire event when clicking on get support * Remove unrelated code * a4a: reorder referrals table and change nume (#97377) * Reader: fix style in tags page sidebar (#97415) * Reader: fix style in tags page sidebar * fix horizontal line too * Paid Stats: Add expandable feature list to upsell (#97367) * Add expandable feature list to upsell * Add new images * Hide overflow in right panel * Add expandable insights upsell * Update style on expand div * Add gradient on upsell right panel * Fix upsell border-radius * Fix upsell image in mobile * Update checkout feature list for stats * Fix for stats rows breaking out --------- Co-authored-by: Rafael Agostini <rafael.agostini@automattic.com> * Stepper: Move conservative lazy loading (#97326) * Enable wrap words when needed (#97339) * Stepper: Fix domains step persistence (#97375) * My Home: Fix alignment on inspiration card call action button (#97422) * Scope the styles for the reader-first-posts__nudge card * Revert "Accessiblity: Fix Help Center portal injection" (#97425) * Remove english gateway (#96502) * Stats: improve tooltips positive/negative trends (#97364) * Stats: refactor 'getNote()' function Provide the note together with the overview data. This way, it will be available regardless of whether the site has paid subscribers, as they are handled differently. * Stats: improve tooltips positive/negative trends * Compared translated strings between notes/labels * Scroll to the top when Checkout loads (#97427) * A4A: Enable in-product feedback on staging and production (#97372) * enable in-product feedback on staging and production * testing change for flaky test * redo testing change for flaky test * change back to intended code. the tests still failed with the noop code conifrming it is unrelated * Revert "Fix Stepper performance bug (#97354)" (#97429) This reverts commit 8c0021b. * Purchases: Convert me/purchases/utils to TS (#97297) * Convert me/purchases/utils to TS * Prevent calling purchase functions without purchase * Export utils functions directly * Add explicit return types to all util functions * composite-checkout: Add paymentMethod ID to errors logged by submit_button_load (#97432) * All date controls up to 30 days for basic plans (#97431) * Goals Screen: Save onboarding goals when creating site (#97426) * include onboarding site goals in the site creation step * gate goal saving behind the experiment feature flag * prevent site goals from being overwritten with empty goals * Stats: Put 30-days summary behind paywall (#97387) * Put 30 days summary behind paywall * Update client/my-sites/stats/hooks/use-should-gate-stats.ts * Update client/my-sites/stats/hooks/use-should-gate-stats.ts Co-authored-by: Dognose <dognose24@users.noreply.github.com> * Add import --------- Co-authored-by: Louis Laugesen <louis.laugesen@gmail.com> Co-authored-by: Dognose <dognose24@users.noreply.github.com> * Goals First experiment should only be assigned in onboarding flow (#97438) * A4A > Agency Tier: Navigate user to the correct section of the KB article (#97385) * Design Picker: Update filter styles (#97393) * Design Picker: Update filter styles * Fix styles on single selection * Remove border from the topic list * Fix the behavior of the responsive toolbar * Design Picker: Update the track event properties (#97379) * Design Picker: Update the track event properties * Update tests * Fix types * Stats: Fix hourly views when the date control is gated (#97437) * Stop forcing date range for period hour * Stop bar clicking when date control is locked * Performance Profiler: Pass translate function to metrics when rendering (#97420) * Sidebar Upsell Nudge: Improve UI (#97278) * Updated the UI according to the design agreed on * Non-dismissible notices do not have a UI change * UI changes now only apply to is-dismissible + is-compact * Minor change in CSS selector * Social Connections: Fix bug with service examples (#97332) * Add all service ID to SERVICES_WITH_EXAMPLES * Add remaining services * Update client/sites/marketing/connections/service-examples.jsx Co-authored-by: Manzoor Wani <manzoorwani.jk@gmail.com> --------- Co-authored-by: Manzoor Wani <manzoorwani.jk@gmail.com> * Design Picker: Dedupe themes shown on the Recommended themes section (#97441) * A4A > Referrals: Implement commissions payment delayed banner (#97443) * Remove GA nudge in stats page (#97447) Co-authored-by: Andrés Blanco <email@gmail.com> * Revert using "Clicks" instead of "Visitors" Since we still use "Clicks" in DSP Widget, changing them to "Visitors" is not as consistent. So we will address that design change later. * Address the case of having less that 1 day Otherwise, for campaigns that were started "today" we will see "0" in the UI which is not appropriate. * Overcome uPlot issue with infinite loop Issue is "Range Error: Invalid array length", leeoniya/uPlot#827. --------- Co-authored-by: Andrija Vučinić <andrija.vucinic@gmail.com> Co-authored-by: Christos <chriskmnds@gmail.com> Co-authored-by: Omar Alshaker <omar@omaralshaker.com> Co-authored-by: Aurorum <43215253+Aurorum@users.noreply.github.com> Co-authored-by: Robert Felty <robfelty@gmail.com> Co-authored-by: Roberto Aranda <roberto.aranda@automattic.com> Co-authored-by: valterlorran <valterlorran@hotmail.com> Co-authored-by: Caroline Moore <calobee@gmail.com> Co-authored-by: Kevin L <40267301+a8ck3n@users.noreply.github.com> Co-authored-by: Aneesh Devasthale <aneeshd16@users.noreply.github.com> Co-authored-by: Konstantin Obenland <obenland@gmx.de> Co-authored-by: Dean Sas <dean.sas@automattic.com> Co-authored-by: dlind1 <dlind1@users.noreply.github.com> Co-authored-by: Payton Swick <payton@foolord.com> Co-authored-by: Andrii Lysenko <60262784+andrii-lysenko@users.noreply.github.com> Co-authored-by: Yashwin Poojary <yashwinpoojary@gmail.com> Co-authored-by: Andrii <lysenkoa.work@gmail.com> Co-authored-by: Travis Walter <travis@automattic.com> Co-authored-by: Rafael Agostini <rafael.agostini@automattic.com> Co-authored-by: Louis Laugesen <louis.laugesen@gmail.com> Co-authored-by: Chi-Hsuan Huang <chihsuan.tw@gmail.com> Co-authored-by: Ilyas Foo <foo.ilyas@gmail.com> Co-authored-by: Ashar Fuadi <ashar.fuadi@automattic.com> Co-authored-by: arthur791004 <arthur.chu@automattic.com> Co-authored-by: Bogdan Nikolic <bogdan.nikolic87@gmail.com> Co-authored-by: Madhu Dollu <madhusudhan.dollu@gmail.com> Co-authored-by: Kosta <heavyweight@users.noreply.github.com> Co-authored-by: Daniel <daniel.lopez@automattic.com> Co-authored-by: Miguel Torres <1233880+mmtr@users.noreply.github.com> Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com> Co-authored-by: Donna Peplinskie <donnapep@gmail.com> Co-authored-by: Christian Gastrell <cgastrell@gmail.com> Co-authored-by: Siddarthan Sarumathi Pandian <siddarthan.sarumathi.pandian@automattic.com> Co-authored-by: Emanuele Buccelli <escapemanuele@gmail.com> Co-authored-by: Eoin Gallagher <eoin@automattic.com> Co-authored-by: John Webb <john.webb@automattic.com> Co-authored-by: Gabriel Caires <gabriel.caires@automattic.com> Co-authored-by: Osk <oskosk@users.noreply.github.com> Co-authored-by: Tiago Ilieve <tiago.ilieve@automattic.com> Co-authored-by: Michael Cain <cain@automattic.com> Co-authored-by: Rafael Gallani <galani.rafael@gmail.com> Co-authored-by: vykes-mac <47489215+vykes-mac@users.noreply.github.com> Co-authored-by: Dognose <dognose24@users.noreply.github.com> Co-authored-by: Philip Jackson <p-jackson@users.noreply.github.com> Co-authored-by: Richard Ortiz <rcrd.ortiz@gmail.com> Co-authored-by: Gergely Márk Juhász <36671565+gmjuhasz@users.noreply.github.com> Co-authored-by: Manzoor Wani <manzoorwani.jk@gmail.com> Co-authored-by: Andrés Blanco <andresblanco@gmail.com> Co-authored-by: Andrés Blanco <email@gmail.com>
Proposed Changes
This PR converts
me/purchases/utils.js
to TypeScript.Testing Instructions
If the automated tests pass, this should be fine.