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

Load the api init script before the tracker so values are populated #355

Merged
merged 3 commits into from Aug 4, 2021

Conversation

@jblz
Copy link
Contributor

@jblz jblz commented Aug 3, 2021

Description

The function that enqueues the tracker script from the CDN is currently firing prior to the one that enqueues the API init script.

This change reverses that and makes explicit via the add_action's $priority param that it should run in that order.

Motivation and Context

The global.PARSELY.onload function has to be populated prior to the tracker being loaded so it can actually be called and the /profile API call conditionally fired.

See:

if ( typeof global.PARSELY.onload !== 'function' ) {
global.PARSELY.onload = uuidProfileCall;
return;
}
const oldonload = global.PARSELY.onload;
global.PARSELY.onload = function() {
if ( oldonload ) {
oldonload();
}
uuidProfileCall();
};
return;
}

How Has This Been Tested?

On a sandboxed version of blog.parse.ly

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)

@jblz jblz self-assigned this Aug 3, 2021
@jblz jblz added this to the 2.6.0 milestone Aug 3, 2021
@jblz jblz marked this pull request as ready for review Aug 3, 2021
@jblz jblz requested a review from Parsely/wp-parsely as a code owner Aug 3, 2021
@GaryJones
Copy link
Contributor

@GaryJones GaryJones commented Aug 4, 2021

What advantage does changing it to priority 9 make? The move to change the order in which it is registered should be enough.

@jblz
Copy link
Contributor Author

@jblz jblz commented Aug 4, 2021

What advantage does changing it to priority 9 make? The move to change the order in which it is registered should be enough.

It makes no functional difference in this change -- it just calls out that it should occur in that order as a visual hint.

If the lines were to be inadvertently reversed e.g in a future refactoring, it would not break the load order.

@GaryJones
Copy link
Contributor

@GaryJones GaryJones commented Aug 4, 2021

So, add a code comment, rather than change the behaviour to something less standard?

@jblz
Copy link
Contributor Author

@jblz jblz commented Aug 4, 2021

done

@jblz jblz merged commit 69bb168 into develop Aug 4, 2021
16 checks passed
16 checks passed
@github-actions
Basic CS and QA checks
Details
@github-actions
Basic CS and QA checks
Details
@github-actions
PHP 5.6
Details
@github-actions
PHP 5.6
Details
@github-actions
PHP 7.0
Details
@github-actions
PHP 7.0
Details
@github-actions
PHP 7.1
Details
@github-actions
PHP 7.1
Details
@github-actions
PHP 7.2
Details
@github-actions
PHP 7.2
Details
@github-actions
PHP 7.3
Details
@github-actions
PHP 7.3
Details
@github-actions
PHP 7.4
Details
@github-actions
PHP 7.4
Details
@github-actions
PHP 8.0
Details
@github-actions
PHP 8.0
Details
@jblz jblz deleted the fix-profile-uuid-call branch Aug 4, 2021
@jblz jblz mentioned this pull request Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants