Skip to content
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

Analytics: Fix recordTracksEvent import in analytics library docs #53526

Merged
merged 1 commit into from
Jun 9, 2021

Conversation

sirbrillig
Copy link
Member

@sirbrillig sirbrillig commented Jun 8, 2021

Changes proposed in this Pull Request

There are two ways to call recordTracksEvent(); one is by using the function directly from the @automattic/calypso-analytics package, and the other is to use the calypso data-layer middleware by dispatching an action to redux. This is a little confusing because both the raw function and the redux action creator are called recordTracksEvents, but they are defined in two different places.

In #41125, an example was added to the README of the @automattic/calypso-analytics package to explain how to use the redux action creator rather than the raw function, but #47453 (accidentally, I think) broke that example. Using the code in the example will in fact throw an error since the exported recordTracksEvents function has no return value and cannot be used as an action creator. This PR restores the correct example, which is not ideal because it references an import path in a separate npm package, but it is better than providing an example that is totally wrong.

Testing instructions

None needed. Just verify that places that use recordTracksEvents inside a redux dispatch call are importing from the location in the corrected example.

Eg:

import { recordTracksEvent } from 'calypso/state/analytics/actions';

@sirbrillig sirbrillig requested a review from scinos June 8, 2021 23:42
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jun 8, 2021
@sirbrillig sirbrillig requested a review from ramonjd June 8, 2021 23:42
@sirbrillig sirbrillig self-assigned this Jun 8, 2021
@matticbot
Copy link
Contributor

@matticbot
Copy link
Contributor

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.

Copy link
Member

@ramonjd ramonjd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I performed a smoke test of a dozen or so places where we're calling dispatch( recordTracksEvent( ... ) );

In all instances, we're importing recordTracksEvent from

import { recordTracksEvent } from 'calypso/state/analytics/actions';

@sirbrillig sirbrillig merged commit 2544768 into trunk Jun 9, 2021
@sirbrillig sirbrillig deleted the fix/analytics-dispatch-docs branch June 9, 2021 14:43
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants