MSD: support CIAB routes at my.woo.localhost and my.woo.com#108550
MSD: support CIAB routes at my.woo.localhost and my.woo.com#108550
Conversation
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
60cc72a to
319de54
Compare
|
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 |
319de54 to
8d7799b
Compare
8d7799b to
a1da870
Compare
p-jackson
left a comment
There was a problem hiding this comment.
Yes I think this is a good track to take. The server continues to decide which AppConfig is getting used, and webpack bundler continues to make a bundle specifically for each MSD variant. So I like that.
I don't like that the hostname config value will be incorrect from the client-side's point of view. In pMz3w-nnj-p2#comment-131646 I suggest also server-rendering the hostname config value based on the incoming request. Ultimately I think we'll need both changes, both server-rendering to make sure hostname is correct, and these changes which choose the correct section based on the incoming request.
| return [ | ||
| 'http://my.localhost:3000', | ||
| 'https://my.wordpress.com', | ||
| 'http://my.woo.localhost:3000', |
There was a problem hiding this comment.
fwiw I kinda like my-woo.localhost:3000. Just to keep the number of sub-domain components consistent.
There was a problem hiding this comment.
Hmm I don't have strong opinion; I think I subconsciously followed the existing jetpack.cloud.localhost hostname. 😄
There was a problem hiding this comment.
Oh, that's fair. TIL about jetpack.cloud.localhost
| boot( { | ||
| name: 'CIAB', | ||
| basePath: '/ciab', | ||
| basePath: getCiabDashboardBasePath( window.location.hostname ), |
There was a problem hiding this comment.
Is this just a temporary thing, having the ability to support both /ciab and my.woo.com? Seems like we should just remove the /ciab path, but this is perhaps to help with the transition so we don't immediately break the link between CIAB site dashboards and the MSD?
There was a problem hiding this comment.
I think we need this anyway for the Dashboard Live link. Since we only have one live environment, and the domain is random string, there's no way to distinguish it. So currently CIAB is accessible at <Dashboard Live link>/ciab/*, while dotcom's is at the root path.
| handleRoute( DASHBOARD_CIAB_SECTION_DEFINITION, '/ciab', 'entry-dashboard-ciab', ( req ) => { | ||
| // Allow CIAB routes under my.localhost. | ||
| return req.get( 'host' ).startsWith( 'my.localhost' ); | ||
| handleRoute( DOTCOM_DASHBOARD_SECTION_DEFINITION, route, 'entry-dashboard-dotcom', ( req ) => |
There was a problem hiding this comment.
Having a separate handleRoute call for each MSD-variant seems fine to me. Calypso v1 has that giant sections.ts file to do it all declaratively, and if we end up with enough MSD-variants maybe we want to do it declaratively too. But I'm not mad about having a copy-pasted line for each dashboard type.
There was a problem hiding this comment.
maybe we want to do it declaratively too.
Hmm, I'm afraid if it's possible at all. We can only get the current request's host when we actually get the request, so that's why we need to "register" all the sections and then filter the correct one based on req.get('host') imperatively here 😬
Part of DOTMSD-1073
Alternative to #108536
Proposed Changes
Add app-side routing to route CIAB hostnames (
my.woo.localhostandmy.woo.com) to serveapp-ciab.The basic idea is that we serve
app-ciabat/base path for the above hostnames, and/ciabfor any other MSD hostnames.Important
We will need to solve how to get the "current dashboard" in this logic, as the above hostnames will no longer have the
/ciabbase path:wp-calypso/client/dashboard/utils/link.ts
Line 118 in 9e300a8
So interaction with Stepper / wpcom could still break. It is out of scope for this PR now.
Why are these changes being made?
See: pMz3w-nnj-p2
Testing Instructions
yarn start:calypso.localhost:3000/sites; verify you see Hosting Dashboard v1calypso.localhost:3000/home/:siteSlug.my.localhost:3000; verify you see MSD (dotcom)my.localhost:3000/ciab; verify you see MSD (CIAB)my.woo.localhost:3000; verify you see MSD (CIAB)yarn start-dashboard:my.localhost:3000; verify you see MSD (dotcom)my.localhost:3000/ciab; verify you see MSD (CIAB)my.woo.localhost:3000; verify you see MSD (CIAB)<Calypso Live>/sites; verify you see Hosting Dashboard v1- Verify you can also visit nav-unification pages like
calypso.localhost:3000/home/:siteSlug.<Dashboard Live>; verify you see MSD (dotcom)<Dashboard Live>/ciab; verify you see MSD (CIAB)Pre-merge Checklist