Stepper: Deprecate goNext, goBack, and goToStep methods

[edanzer]

Proposed Changes

Resolves: https://github.com/Automattic/wp-calypso/issues/68165

This PR makes three changes:

• Makes two Stepper methods (goNext, goBack) optional in NavigationControls type.
• Adds JSDocs deprecation notices to goNext, goBack, and goToStep methods in the NavigationControls type.
• Removes these methods from the useStepNavigation hook in the link-in-bio-post-setup flow. These methods are not used in this flow. But before, removing them would have generated TypeScript errors because the methods were required as part of the NavigationsControls type.

Testing Instructions

The methods in question are still valid, and this PR should not affect any code logic. The methods should simply show as deprecated when they are used in code.

1. Checkout this branch and run yarn and yarn start if needed.

2. Confirm that methods are deprecated. First go to the client/landing/stepper/declarative-flow/internals/types.ts, hover over the deprecated types, and you should see a deprecated notice like this:

3. Where these methods are actually used in current stepper screens, if VS Code is configured to show JSDoc deprecation, the methods will now have a strikethrough. It looks like this on the launchpad index.ts screen:

4. Confirm that there are no TypeScript errors in client/landing/stepper/declarative-flow/link-in-bio-post-setup.ts where the methods have been deleted from the useStepNavigation hook.

5. Confirm that there are no other TypeScript errors in files where these methods are still used that would prevent TS compiling.

6. Test the Newsletter flow, which uses the deprecated methods, to confirm no regressions or breakages. Note we may want to refactor flows to avoid using these methods, but for now, we should confirm that all flows still work as expected

• Go to https://wordpress.com/hp-2022-tailored-flows/ and choose the Newsletter flow.
• On one of early screens, replace https://wordpress.com with http://calypso.localhost:3000 to ensure you are running your local branch.
• Go through the flow until the Launchpad screen. Click the Add Subscribers task (uses deprecated goToStep method), and confirm you are properly redirected to the Add Subscribers page.
• Navigate back to Launchpad and click the Go to Admin screen (uses deprecated goNext method) and confirm you are properly redirected to admin.

[github-actions[bot]]

Calypso Live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-50548

Jetpack Cloud live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-50548&env=jetpack

[matticbot]

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

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

name           parsed_size           gzip_size
entry-stepper        -39 B  (-0.0%)      +22 B  (+0.0%)

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

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.

[jeyip]

I missed a lot of contextual conversation for this decision, and this might be a silly question, but if a user wants to return to a previous screen to update answers they've provided, how would they do so?

[jeyip]

Testing

Requirements

• [x] Confirm that methods are deprecated in vscode and there is a deprecated notice
• [x] Use of deprecated methods have a strikethrough
• [x] No TypeScript errors in client/landing/stepper/declarative-flow/link-in-bio-post-setup.ts
• [x] No general TypeScript errors that I could find in files where these methods are used
• [x] Click the Add Subscribers task (uses deprecated goToStep method), and confirm you are properly redirected to the Add Subscribers page.
• [x] Click the Go to Admin screen (uses deprecated goNext method) and confirm you are properly redirected to admin.

Browsers

• [x] Chrome

[jeyip]

+1 from me if it looks good to @alshakero

[alshakero]

I missed a lot of contextual conversation for this decision

The reasoning behind this is to remove any interaction between steps. The biggest source of problems in /start is the undocumented (frankly undocumentable) interaction between steps, so having the steps be as agnostic to their position in the flow as possible as a new approach that we're testing. This makes the step just a fancy form component that just submits what it wants. And leaves the flow to decide where to go next based on what the step submits + the state in the store. We're making the flows strictly uni-directional. Similar to Redux way of thinking.

goBack and goNext are a leak in that paradigm. Because they imply that the step knows its place in the flow. And this makes the steps buggy + less reusable.

I'll document all of this soon.

and this might be a silly question, but if a user wants to return to a previous screen to update answers they've provided, how would they do so?

Not silly at all! My initial idea was either the flow would take care of taking the user back by popping the history stack, or by the step submitting that it wants to go back and flow taking it back. My doubt about the second option is that the flow may push another step in history, when it needs to pop.

I'll play with this and come back to this PR.

[jeyip]

The reasoning behind this is to remove any interaction between steps. The biggest source of problems in /start is the undocumented (frankly undocumentable) interaction between steps, so having the steps be as agnostic to their position in the flow as possible as a new approach that we're testing. This makes the step just a fancy form component that just submits what it wants. And leaves the flow to decide where to go next based on what the step submits + the state in the store. We're making the flows strictly uni-directional. Similar to Redux way of thinking.

goBack and goNext are a leak in that paradigm. Because they imply that the step knows its place in the flow. And this makes the steps buggy + less reusable.

I'll document all of this soon.

Thanks for elaborating! Much appreciated 🙏

[jeyip]

Non-urgent, friendly ping to see where we're at here @edanzer + @alshakero

[agrullon95]

Tests:

The changes look good to me. I don't see any TS errors in the changed files and the newsletter flow has no issues.

• [x] Confirm deprecated method notices are appearing when I hover over methods.
• [x] Confirm that there are no TypeScript errors in client/landing/stepper/declarative-flow/link-in-bio-post-setup.ts where the methods have been deleted from the useStepNavigation hook.
• [x] Confirm that there are no other TypeScript errors in files where these methods are still used that would prevent TS compiling.
• [x] Test the Newsletter flow, which uses the deprecated methods, to confirm no regressions or breakages. Test Add subscribers task and Go to Admin link.