Remove legacy data API and legacy higher-order components

[felixarntz]

The legacy data API and the legacy higher-order components withData and addToFilter should be removed once they are no longer used.

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

• assets/js/components/data should be entirely removed. Before, it should be verified it is no longer used anywhere.
• assets/js/components/higherorder should be entirely removed. Before, it should be verified it is no longer used anywhere.
• The legacy infrastructure for mocking Storybook data should be entirely removed. E.g.:
  • .storybook/data/*
  • assets/js/util/cache-data.js
• Any Storybook stories that use data from .storybook/data/* should be removed. Those should be individually listed in the IB, so that decisions whether they can just be removed or whether they need to be replaced can be made.

Implementation Brief

1. Remove assets/js/components/data

collect-module-data.js

• Remove the file
• Then, in assets/js/components/Root/index.js:
  • Remove <CollectModuleData>
  • Remove legacy dataAPI* props
  • Go through the consumers of <Root>, find instances where these props are passed, and remove them:
    • assets/js/googlesitekit-user-input.js
    • assets/js/googlesitekit-dashboard.js
    • assets/js/googlesitekit-dashboard-splash.js
    • assets/js/googlesitekit-activation.js
    • assets/js/googlesitekit-adminbar.js
    • assets/js/googlesitekit-settings.js
    • assets/js/googlesitekit-dashboard-details.js
    • assets/js/googlesitekit-module.js
    • assets/js/googlesitekit-wp-dashboard.js

invalidate-cache-group.js

• Remove the file and all usage from the following:
  • assets/js/modules/search-console/datastore/settings.js
  • assets/js/modules/tagmanager/datastore/settings.js
  • assets/js/modules/analytics/datastore/settings.js
  • assets/js/googlesitekit/data/create-settings-store.js
• double check the codebase and remove any remaining instances.

constants.js
Remove the file, and delete the imports from:

• assets/js/modules/analytics/datastore/settings.js
• assets/js/modules/tagmanager/datastore/settings.js
• assets/js/modules/search-console/datastore/settings.js
• assets/js/googlesitekit/data/create-settings-store.js

cache.js
This is used by legacy-notifications, but we should be able to substitute calls to the legacy data API with their new counterparts throughout.

• go through assets/js/components/legacy-notifications/site/mark-notification.js and assets/js/components/legacy-notifications/util.js
• replace calls to data.set/ get with the counterpart calls to the new googlesitekit-api
• remove cache imports and replace uses with the modern counterpart from googlesitekit/api/cache
• remove invalidateCacheGroup imports and replace uses with the modern counterpart from googlesitekit/api/cache

index.js
The imports can be removed from these files:

• assets/js/googlesitekit-wp-dashboard.js
• assets/js/googlesitekit-module.js

/test and /utils

• Remove the test and utils directories.

2. Remove assets/js/components/higherorder/

• Double check the codebase, but after Launch Widget API-based dashboard without feature flag #1997 none of these should be in use. We should be good to remove all of them.
• Do not remove withFeatureFlag.js, however (see Remove legacy data API and legacy higher-order components #2258 (comment)).

3. Storybook legacy data

.storybook/data/*
The following files are not used any more and can be removed:

  blog---googlesitekit.js
  blog---googlesitekitAdminbar.js
  wp-admin-admin.php-page=googlesitekit-module-adsense-googlesitekit.js
  wp-admin-admin.php-page=googlesitekit-module-adsense-googlesitekitCurrentModule.js
  wp-admin-admin.php-page=googlesitekit-module-adsense-googlesitekitPerformanceModule.js
  wp-admin-admin.php-page=googlesitekit-module-analytics-googlesitekitCurrentModule.js
  wp-admin-admin.php-page=googlesitekit-module-analytics-googlesitekit.js
  wp-admin-admin.php-page=googlesitekit-module-search-console-googlesitekit.js
  wp-admin-admin.php-page=googlesitekit-module-search-console-googlesitekitCurrentModule.js
  wp-admin-admin.php-page=googlesitekit-splash-googlesitekit.js
  wp-admin-index.php--googlesitekit.js

These two files are in use and need an additional step before removal (see #2258 (comment) )

1. wp-admin-admin.php-page=googlesitekit-dashboard-googlesitekit.js
   • Used by .storybook/utils/resetGlobals.js
2. wp-admin-admin.php-page=googlesitekit-settings-googlesitekit.js
   • Used by stories/settings.stories.js

In both these cases:

• Determine whether removing the referenced blob of data has any impact on the story / stories in question
  • if not, remove the references and the data file
  • if so, hard-code the blob of data into the story file and then remove the data file

Test Coverage

• No updates anticipated

Visual Regression Changes

• None anticipated

QA Brief

• Verify that the ACs have been met and that the files mentioned have been removed.
• There should be no regressions in storybook (in particular the stories produced by stories/settings.stories.js).
• Perform a quick regression test on the Site Kit app to verify there are no major breakages. Areas to focus on are those that use:
  • legacy-setup
  • legacy-notifications

Changelog entry

• Remove legacy data API code.

[aaemnnosttv]

@felixarntz we have at least one HOC in assets/js/components/higherorder which shouldn't be removed so we should probably update the ACs to be specific to the individual legacy components.

[johnPhillips]

Hi @felixarntz, just a couple of questions:

Regarding assets/js/components/data/constants.js,TYPE_MODULES seems to be in use in a couple of places:

• assets/js/modules/analytics/datastore/settings.js
• assets/js/modules/tagmanager/datastore/settings.js
• assets/js/modules/search-console/datastore/settings.js
• assets/js/googlesitekit/data/create-settings-store.js

What would you like done here - should I remove the constants file and inline all the references, for example?

Also, regarding this:

Any Storybook stories that use data from .storybook/data/* should be removed. Those should be individually listed in the IB, so that decisions whether they can just be removed or whether they need to be replaced can be made.

I think almost everything in there is unused, however these two files are used:

1. wp-admin-admin.php-page=googlesitekit-dashboard-googlesitekit.js
   • Used by .storybook/utils/resetGlobals.js
2. wp-admin-admin.php-page=googlesitekit-settings-googlesitekit.js
   • Used by stories/settings.stories.js

I don't have a huge amount of context on this legacy stuff 😅 - how would you like me to approach these?

[felixarntz]

@johnPhillips
The parts you're referring to related to assets/js/components/data usage within the modules should be completely removed - see the TODO comments, they are only still there because the data API is still in the codebase (which is being changed in this issue).

Regarding .storybook/utils/resetGlobals.js: I think for now the most straightforward change is to take the referenced blob of data from the file to remove and hard-code it into the resetGlobals function. We can later review whether any of the fields in there are unused.

It looks like stories/settings.stories.js does a similar thing. Since globals are reset for every Storybook story anyway, I think you can just remove the respective assignment/usage entirely. If there is any nuance between the settingsData compared to the dashboardData which causes the Storybook story to behave differently, you can manually assign the different pieces of data in that Admin Settings story.

Per the above three, you should then be able to use the legacy API and its Storybook data mocks completely.

[aaemnnosttv]

@johnPhillips – looks great, just a few points to tweak

cache.js

• remove cache imports and uses
• remove invalidateCacheGroup imports and uses

These would also be replaced with modern counterparts from googlesitekit/api/cache, not simply removed (unless of course they were simply no longer used, but I don't think that's the case).

assets/js/util/cache-data.js

This file has already been removed 😄

Otherwise, this looks good to go!

[johnPhillips]

Thanks @aaemnnosttv - IB updated 👍

[aaemnnosttv]

IB ✅

[aaemnnosttv]

Let's make sure this one get's both QA:Eng and regular QA for a thorough pass. @johnPhillips would you please update the QAB to include a section for both (one part for eng, the other QA)?

[felixarntz]

@johnPhillips @aaemnnosttv Going to remove this from the 1.38.0 release since it's a lot of changes and not critical for user-facing functionality. Having some extra time to use it once merged makes sense.

[aaemnnosttv]

Taking over here to finish this up as John is out for a bit.

[eugene-manuilov]

QA ⚠️ I know that QAB says that we don't need to delete the withFeatureFlag HOC, but it is no longer in use, so we can get rid of it too.

[tofumatt]

Fair enough—I've made a follow-up PR at #3793 👍🏻

[eugene-manuilov]

QA ✔️