Add real-time comments to Reader full-post view #40396

mhsdef · 2020-03-24T15:00:03Z

We're getting our Calypso "sea legs" back under us. And very open to detailed integration input as this gets off the ground.

Make it work.
Make it beautiful. <--here.
Make it fast (if you need to, "90% of the time" step two has you covered).

Changes proposed in this Pull Request

Add Phoenix-based websocket middleware.
Add real-time comments to Reader full-post view behind a feature flag.
Disable Reader comment polling while we explore the space.

Testing instructions

Apply D40881-code to your sandbox.
Fire up this branch locally or hit calypso.live.
Go to a full post view in the Reader on a public post. View in one window and comment in another. Comment from traditional WP.com. Comment from Notifs.

You should see the Reader update real-time in all cases.

matticbot · 2020-03-24T15:00:05Z

Test live: https://calypso.live/?branch=try/realtime-comments

blowery · 2020-03-24T15:06:04Z

Amazing!

What's lasagna?

mhsdef · 2020-03-24T15:26:51Z

A backend try piece, over here: https://github.com/Automattic/lasagna

matticbot · 2020-03-24T16:25:04Z

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Webpack Runtime (~77 bytes removed 📉 [gzipped])

name      parsed_size           gzip_size
manifest      -1647 B  (-0.9%)      -77 B  (-0.2%)

Webpack runtime for loading modules. It is included in the HTML page as an inline script. Is downloaded and parsed every time the app is loaded.

App Entrypoints (~12225 bytes added 📈 [gzipped])

name                   parsed_size           gzip_size
entry-main                +53857 B  (+3.7%)   +13040 B  (+3.5%)
entry-domains-landing       -142 B  (-0.0%)     -531 B  (-0.3%)
entry-gutenboarding          +99 B  (+0.0%)     -277 B  (-0.0%)
entry-login                  +21 B  (+0.0%)       -7 B  (-0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~27675 bytes removed 📉 [gzipped])

name                    parsed_size            gzip_size
reader                     -52213 B  (-11.2%)   -12828 B  (-10.6%)
comments                   -26759 B   (-5.4%)    -6835 B   (-5.9%)
site-blocks                -25800 B   (-9.2%)    -6598 B   (-9.0%)
phoenix                    +22419 B     (new)    +6409 B     (new)
gutenberg-editor            -3892 B   (-0.5%)     -464 B   (-0.2%)
checkout                    -3725 B   (-0.3%)     +199 B   (+0.1%)
stats                       -2673 B   (-0.3%)     -597 B   (-0.3%)
post-editor                 -1870 B   (-0.1%)     -958 B   (-0.2%)
media                       -1740 B   (-0.5%)     -327 B   (-0.4%)
activity                    -1654 B   (-0.4%)     -354 B   (-0.3%)
woocommerce                 -1554 B   (-0.1%)     -326 B   (-0.1%)
pages                       -1517 B   (-0.8%)     -324 B   (-0.6%)
plugins                     -1465 B   (-0.4%)     -333 B   (-0.3%)
help                        -1334 B   (-0.3%)     -410 B   (-0.4%)
posts-custom                -1217 B   (-0.4%)     -271 B   (-0.3%)
posts                       -1217 B   (-0.4%)     -271 B   (-0.3%)
themes                      -1150 B   (-0.4%)     -240 B   (-0.3%)
theme                       -1150 B   (-0.5%)     -240 B   (-0.4%)
settings-security           -1150 B   (-0.5%)     -233 B   (-0.4%)
settings-performance        -1150 B   (-0.5%)     -240 B   (-0.4%)
marketing                   -1125 B   (-0.3%)     -221 B   (-0.2%)
purchases                   -1113 B   (-0.1%)     -193 B   (-0.1%)
zoninator                   -1050 B   (-0.4%)     -257 B   (-0.4%)
domains                     -1030 B   (-0.1%)     +498 B   (+0.2%)
jetpack-connect              -980 B   (-0.2%)     -141 B   (-0.1%)
settings                     -966 B   (-0.2%)     -217 B   (-0.2%)
plans                        -943 B   (-0.2%)     -116 B   (-0.1%)
account                      -850 B   (-0.3%)     -187 B   (-0.2%)
earn                         -831 B   (-0.3%)     -161 B   (-0.2%)
hosting                      -813 B   (-0.4%)     -153 B   (-0.3%)
home                         -750 B   (-0.3%)     -169 B   (-0.2%)
people                       -646 B   (-0.2%)     -141 B   (-0.2%)
export                       -646 B   (-0.4%)      -87 B   (-0.2%)
customize                    -646 B   (-0.5%)     -116 B   (-0.3%)
backups                      -646 B   (-0.2%)     -122 B   (-0.2%)
settings-writing             -629 B   (-0.1%)     -146 B   (-0.1%)
email                        -526 B   (-0.2%)     +652 B   (+0.9%)
notification-settings        -504 B   (-0.2%)     -112 B   (-0.1%)
sites                        -346 B   (-0.5%)      -66 B   (-0.3%)
settings-discussion          -346 B   (-0.2%)      -56 B   (-0.1%)
sensei                       -346 B   (-0.4%)      -64 B   (-0.3%)
scan                         -346 B   (-0.3%)      -59 B   (-0.2%)
preview                      -346 B   (-0.5%)      -66 B   (-0.3%)
migrate                      -346 B   (-0.3%)      -58 B   (-0.2%)
jetpack-cloud-settings       -346 B   (-0.3%)      -64 B   (-0.2%)
import                       -346 B   (-0.2%)      -64 B   (-0.1%)
hello-dolly                  -346 B   (-0.4%)      -62 B   (-0.2%)
google-my-business           -346 B   (-0.1%)      -58 B   (-0.1%)
feature-upsell               -346 B   (-0.3%)      -62 B   (-0.2%)
concierge                    -346 B   (-0.1%)      -70 B   (-0.1%)
checklist                    -346 B   (-0.2%)      -69 B   (-0.1%)
wp-super-cache               -309 B   (-0.1%)      -35 B   (-0.1%)
signup                       -300 B   (-0.2%)      -62 B   (-0.1%)
account-close                -300 B   (-0.1%)      -85 B   (-0.1%)
accept-invite                -300 B   (-0.1%)      -64 B   (-0.1%)
devdocs                       +65 B   (+0.0%)       -4 B   (-0.0%)
security                      +37 B   (+0.0%)      +23 B   (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~39984 bytes removed 📉 [gzipped])

name                                                         parsed_size            gzip_size
async-load-components-web-preview-component                     -52575 B  (-12.2%)   -12893 B  (-11.6%)
async-load-design-blocks                                        -52220 B   (-1.9%)   -11982 B   (-1.8%)
async-load-blocks-support-article-dialog-dialog                 -27173 B  (-31.1%)    -7061 B  (-28.0%)
async-load-signup-steps-domains                                  -1119 B   (-0.5%)     -248 B   (-0.5%)
async-load-post-editor-media-modal                                -999 B   (-0.4%)     +177 B   (+0.3%)
async-load-signup-steps-site-picker                               -961 B   (-2.1%)     -220 B   (-1.8%)
async-load-signup-steps-plans-atomic-store                        -949 B   (-0.8%)     -204 B   (-0.7%)
async-load-signup-steps-plans                                     -949 B   (-0.6%)     -223 B   (-0.5%)
async-load-blocks-inline-help-popover                             -646 B   (-0.3%)     -108 B   (-0.2%)
async-load-signup-steps-wp-for-teams-site                         -615 B   (-2.4%)     -156 B   (-2.5%)
async-load-signup-steps-user                                      -615 B   (-0.5%)     -153 B   (-0.5%)
async-load-signup-steps-theme-selection                           -615 B   (-1.0%)     -135 B   (-0.8%)
async-load-signup-steps-test-step                                 -615 B   (-7.7%)     -168 B   (-7.3%)
async-load-signup-steps-survey                                    -615 B   (-3.7%)     -145 B   (-3.3%)
async-load-signup-steps-site-type                                 -615 B   (-5.4%)     -165 B   (-5.2%)
async-load-signup-steps-site-topic                                -615 B   (-2.2%)     -152 B   (-1.9%)
async-load-signup-steps-site-title                                -615 B   (-5.5%)     -163 B   (-5.3%)
async-load-signup-steps-site-style                                -615 B   (-2.1%)     -175 B   (-1.8%)
async-load-signup-steps-site-or-domain                            -615 B   (-3.2%)     -158 B   (-2.9%)
async-load-signup-steps-site                                      -615 B   (-2.5%)     -155 B   (-2.5%)
async-load-signup-steps-rewind-were-backing                       -615 B   (-7.1%)     -168 B   (-6.5%)
async-load-signup-steps-rewind-migrate                            -615 B   (-2.5%)     -167 B   (-2.5%)
async-load-signup-steps-rewind-form-creds                         -615 B   (-1.6%)     -167 B   (-1.6%)
async-load-signup-steps-rebrand-cities-welcome                    -615 B   (-4.7%)     -166 B   (-4.6%)
async-load-signup-steps-reader-landing                            -615 B   (-5.4%)     -169 B   (-5.4%)
async-load-signup-steps-passwordless                              -615 B   (-2.5%)     -159 B   (-2.6%)
async-load-signup-steps-import-url-onboarding                     -615 B   (-1.8%)     -150 B   (-1.6%)
async-load-signup-steps-import-url                                -615 B   (-2.8%)     -151 B   (-2.5%)
async-load-signup-steps-import-preview                            -615 B   (-2.5%)     -158 B   (-2.2%)
async-load-signup-steps-creds-permission                          -615 B   (-2.7%)     -156 B   (-2.5%)
async-load-signup-steps-creds-confirm                             -615 B   (-2.7%)     -156 B   (-2.5%)
async-load-signup-steps-creds-complete                            -615 B   (-7.1%)     -166 B   (-6.4%)
async-load-signup-steps-clone-start                               -615 B   (-4.4%)     -169 B   (-4.1%)
async-load-signup-steps-clone-ready                               -615 B   (-2.2%)     -169 B   (-2.2%)
async-load-signup-steps-clone-point                               -615 B   (-0.4%)     -136 B   (-0.4%)
async-load-signup-steps-clone-jetpack                             -615 B   (-5.9%)     -167 B   (-5.9%)
async-load-signup-steps-clone-destination                         -615 B   (-3.1%)     -166 B   (-3.2%)
async-load-signup-steps-clone-credentials                         -615 B   (-1.1%)     -152 B   (-1.0%)
async-load-signup-steps-clone-cloning                             -615 B   (-4.5%)     -170 B   (-4.3%)
async-load-signup-steps-about                                     -615 B   (-1.1%)     -152 B   (-0.9%)
async-load-layout-guided-tours                                    -615 B   (-3.5%)     -148 B   (-3.6%)
async-load-docs-selectors                                         -615 B  (-11.0%)     -172 B   (-9.8%)
async-load-blocks-inline-help                                     -615 B   (-1.0%)     -144 B   (-1.1%)
async-load-design                                                 -595 B   (-0.0%)     -781 B   (-0.2%)
async-load-design-playground                                      -537 B   (-0.0%)      -94 B   (-0.0%)
async-load-my-sites-current-site-notice                           -504 B   (-1.1%)     -110 B   (-1.0%)
async-load-blocks-jitm-templates-sidebar-banner                   -504 B   (-3.2%)     -145 B   (-3.1%)
async-load-blocks-jitm-templates-notice                           -504 B   (-3.1%)     -148 B   (-3.1%)
async-load-blocks-jitm-templates-default                          -504 B   (-4.6%)     -142 B   (-4.3%)
async-load-layout-masterbar-drafts-popover                        -404 B   (-1.4%)     -116 B   (-1.3%)
async-load-my-sites-checklist-wpcom-checklist-component-jsx       -367 B   (-0.5%)      -93 B   (-0.5%)
async-load-components-sites-popover                               -346 B   (-0.7%)      -66 B   (-0.5%)
async-load-my-sites-current-site-stale-cart-items-notice          -300 B   (-0.8%)      -61 B   (-0.5%)
async-load-apps-notifications-index-jsx                           -300 B   (-0.3%)      -68 B   (-0.2%)
async-load-blocks-support-article-dialog                          -180 B  (-17.6%)      -26 B   (-5.8%)
async-load-blocks-reader-full-post                                +102 B   (+0.2%)      +14 B   (+0.1%)
async-load-my-sites-current-site-domain-warnings                   +37 B   (+0.1%)      +26 B   (+0.2%)
async-load-lib-preferences-helper                                  -11 B   (-0.1%)       -9 B   (-0.2%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

blowery · 2020-03-24T16:26:45Z

@hewsut cooooool

a8ci18n · 2020-03-31T05:31:39Z

client/blocks/presence-button/index.jsx

+
+			{ showLabel && (
+				<span className="presence-button__label-status">
+					{ translate( 'Viewing', 'Viewing', {


ℹ️ This string doesn't exist elsewhere without a context (or with a different one). You can probably drop the context. If needed, you can use a translator comment instead to clarify meaning.

mhsdef · 2020-04-03T19:19:10Z

I am not sure why ICFY seems to report the app entry endpoint still +12 KB. Maybe I did the dynamic import wrong?

Ready for eyes either way.

To come in follow up PR

griffbrad

I believe a webpackChunkName comment should ensure the dynamic phoenix import is always included in a separate, async-loaded chunk. See https://webpack.js.org/guides/code-splitting/#dynamic-imports

mhsdef · 2020-04-07T16:55:26Z

@griffbrad I thought I had this nailed in 7277061 after your feedback but ICFY still seems to report +12 KB (in entry-main).

FWIW I think I see it coming down as a separate JS chunk now only in Reader:

So I'm not sure what I'm missing again.

griffbrad · 2020-04-07T22:34:03Z

@sgomes have you encountered this before? Bundle size on entry-main is inflated in ICFY but the dependency in question is being loaded async in dev tools.

unDemian · 2020-04-08T09:20:21Z

My guess is the 12kb is not from loading Phoenix library which seems to be correctly lazy loaded and only be around 6kb (https://d.pr/i/7WVJiT).
I think it might be the actual lasagna middleware and its dependencies which is always loaded

wp-calypso/client/state/index.js

Line 71 in 0a3aa22

    
           isBrowser && config.isEnabled( 'lasagna' ) && require( './lasagna/middleware.js' ).default,

Subsequent PRs built on top of this one #40720 #40854 show an increase since they add more code tied to that middleware.

Here is an identical branch which doesn't load the middleware at all Test/bench #40883, entry-main is almost untouched.
Here is an identical branch which loads the middleware code but doesn't load phoenix at all
Test/bench 2 #40889, entry-main is affected by the same amount.

I'm not quite sure how we can fix this, I guess we would need some kind of async middleware registration if we want to conditionally load the middleware code only when needed, I bet there is some kind of way of doing that in Calypso 🤷‍♂️

sgomes · 2020-04-08T11:52:29Z

Excellent research, @unDemian! I agree with your assessment; the middleware appears to be at fault here. I don't think there is currently any example of an asynchronously-loaded middleware anywhere in Calypso, but it should be possible, at least in theory. In practice, there may need to be some changes to the state layer in order to allow for it 😕

Unfortunately, any middleware approach is prone to these sorts of issues, since it separates functionality from the place where it's used, and makes it available everywhere. I've been looking at similar issues with analytics (which has its own middleware), as I'm trying to avoid the whole thing being loaded every time, regardless of what's in use.

Edit: I'm currently trying to analyse the bundles before/after this change to confirm the above findings and narrow in on the extra code being loaded. I'll post a new comment when I have this.

sirreal · 2020-04-08T11:58:35Z

client/state/lasagna/actions.js

+export const socketConnected = () => ( { type: 'LASAGNA_SOCKET_CONNECTED' } );
+export const socketDisconnected = () => ( { type: 'LASAGNA_SOCKET_DISCONNECTED' } );


The action types are added to client/state/action-types but plain strings are used here.

That should be fixed in the next PR #40720

unDemian · 2020-04-08T12:23:54Z

Unfortunately, any middleware approach is prone to these sorts of issues, since it separates functionality from the place where it's used, and makes it available everywhere. I've been looking at similar issues with analytics (which has its own middleware), as I'm trying to avoid the whole thing being loaded every time, regardless of what's in use.

I guess having a "placeholder" middleware that allows dynamic add/remove would do the trick, something like https://github.com/pofigizm/redux-dynamic-middlewares maybe?

lezama · 2020-04-08T12:38:13Z

@griffbrad @sgomes

Two questions (or three or four)!

We are talking about just +3.7% increase, right? could we go ahead merge it and fix the middleware async loading later 😅?
Other than bundle size, what do you think about the code and approach? Ready to merge? 😃

griffbrad

OK with merging now and circling back on the perf concerns, yes. Sergio and I are discussing more and he's graciously offered to help with the middleware perf. The overall approach seems solid.

mhsdef · 2020-04-08T16:50:56Z

Thanks all. I'm going to merge then whenever all these tests clear. Andrei has three or four right behind this that will improve immediately.

Add Lasagna websocket middleware (first pass)

5a63490

mhsdef added [Status] In Progress [Feature] Reader The reader site on Calypso. [Feature] Comments Comments on posts and the admin screen for managing them. labels Mar 24, 2020

mhsdef requested a review from a team as a code owner March 24, 2020 15:00

mhsdef self-assigned this Mar 24, 2020

mhsdef added the DO NOT MERGE label Mar 24, 2020

mhsdef added 5 commits March 24, 2020 23:40

Step forward

74f6fa3

Sync to latest changes

3a41869

One other file

be2ee42

Fix topics

c8f2ce2

Check in a bunch of stuff

d8f2713

mhsdef requested a review from a team as a code owner March 31, 2020 05:27

a8ci18n reviewed Mar 31, 2020

View reviewed changes

mhsdef added 7 commits April 1, 2020 17:10

Remove Presence for now

f0e7ee9

Fix logging path

e511e86

Remove Presence stragglers

f496ca8

More Presence stragglers

d02422c

More cleanup

6e105c6

Merge branch 'master' into try/realtime-comments

d4324ba

Iterate

6e23185

mhsdef added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed DO NOT MERGE [Status] In Progress labels Apr 2, 2020

mhsdef requested a review from a team April 2, 2020 03:19

Merge branch 'master' into try/realtime-comments

ed5149e

lezama requested a review from griffbrad April 6, 2020 12:28

mhsdef and others added 4 commits April 6, 2020 08:54

Merge branch 'master' into try/realtime-comments

aebbe1b

Remove stray addition

dde8e3b

To come in follow up PR

Add config.isEnabled check back for lasagna

76a83fa

Merge branch 'master' into try/realtime-comments

5790016

griffbrad requested changes Apr 7, 2020

View reviewed changes

mhsdef added 2 commits April 7, 2020 12:23

Merge branch 'master' into try/realtime-comments

3be94fe

Add webpackchunkname to dynamic import

7277061

To be handled in a subsequent PR

be75f4f

Merge branch 'master' into try/realtime-comments

0a3aa22

sirreal reviewed Apr 8, 2020

View reviewed changes

griffbrad approved these changes Apr 8, 2020

View reviewed changes

mhsdef added 2 commits April 8, 2020 11:30

Final adjustment

5b82179

Merge branch 'master' into try/realtime-comments

19b1dae

mhsdef merged commit 5b38c63 into master Apr 8, 2020

matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Apr 8, 2020

unDemian mentioned this pull request Apr 9, 2020

Load middleware async #40913

Merged

aaronrobertshaw mentioned this pull request Apr 21, 2020

Suggested article links hanging up #41061

Closed

bluefuton mentioned this pull request Jul 26, 2020

Reader: poll for changes in the comment list #792

Closed

mhsdef added the Real-time label Sep 15, 2020

sirreal deleted the try/realtime-comments branch November 27, 2020 11:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add real-time comments to Reader full-post view #40396

Add real-time comments to Reader full-post view #40396

mhsdef commented Mar 24, 2020 •

edited

Loading

matticbot commented Mar 24, 2020

blowery commented Mar 24, 2020

mhsdef commented Mar 24, 2020 •

edited

Loading

matticbot commented Mar 24, 2020 •

edited

Loading

blowery commented Mar 24, 2020

a8ci18n Mar 31, 2020

mhsdef commented Apr 3, 2020

griffbrad left a comment

mhsdef commented Apr 7, 2020 •

edited

Loading

griffbrad commented Apr 7, 2020

unDemian commented Apr 8, 2020 •

edited

Loading

sgomes commented Apr 8, 2020 •

edited

Loading

sirreal Apr 8, 2020

unDemian Apr 8, 2020

unDemian commented Apr 8, 2020

lezama commented Apr 8, 2020

griffbrad left a comment

mhsdef commented Apr 8, 2020

		export const socketConnected = () => ( { type: 'LASAGNA_SOCKET_CONNECTED' } );
		export const socketDisconnected = () => ( { type: 'LASAGNA_SOCKET_DISCONNECTED' } );

Add real-time comments to Reader full-post view #40396

Add real-time comments to Reader full-post view #40396

Conversation

mhsdef commented Mar 24, 2020 • edited Loading

Changes proposed in this Pull Request

Testing instructions

matticbot commented Mar 24, 2020

blowery commented Mar 24, 2020

mhsdef commented Mar 24, 2020 • edited Loading

matticbot commented Mar 24, 2020 • edited Loading

blowery commented Mar 24, 2020

a8ci18n Mar 31, 2020

Choose a reason for hiding this comment

mhsdef commented Apr 3, 2020

griffbrad left a comment

Choose a reason for hiding this comment

mhsdef commented Apr 7, 2020 • edited Loading

griffbrad commented Apr 7, 2020

unDemian commented Apr 8, 2020 • edited Loading

sgomes commented Apr 8, 2020 • edited Loading

sirreal Apr 8, 2020

Choose a reason for hiding this comment

unDemian Apr 8, 2020

Choose a reason for hiding this comment

unDemian commented Apr 8, 2020

lezama commented Apr 8, 2020

griffbrad left a comment

Choose a reason for hiding this comment

mhsdef commented Apr 8, 2020

mhsdef commented Mar 24, 2020 •

edited

Loading

mhsdef commented Mar 24, 2020 •

edited

Loading

matticbot commented Mar 24, 2020 •

edited

Loading

mhsdef commented Apr 7, 2020 •

edited

Loading

unDemian commented Apr 8, 2020 •

edited

Loading

sgomes commented Apr 8, 2020 •

edited

Loading