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

Add data-cfasync="false" to script tags #286

Merged
merged 4 commits into from
May 6, 2021
Merged

Conversation

jblz
Copy link
Contributor

@jblz jblz commented May 5, 2021

Description

The current version of the plugin adds a data-cfasync="false" attribute pair to the script tags:

<script data-cfasync="false">
function uuidProfileCall() {

<script data-cfasync="false" id="parsely-cfg" data-parsely-site="<?php echo esc_attr( $parsely_options['apikey'] ); ?>" src="//cdn.parsely.com/keys/<?php echo esc_attr( $parsely_options['apikey'] ); ?>/p.js"></script>

<script data-cfasync="false">

This change:

  • Adds the API script to the list of handles that gets the treatment
  • Fixes the preg_replace to actually work
  • (minor) changes a short array syntax to long to match existing code style

Motivation and Context

It was intended to replicate this behavior in #236, but it did not make it into the change.

I didn't find a whole lot of context around the need for this, but there's #129. @hbbtstar -- can you please take a look and let me know if this is the right approach?

Some more context: this attribute looks like it's used to have Cloudflare's Rocket Loader "ignore specific JavaScripts."

How Has This Been Tested?

  • Automated tests added to confirm the attribute is correctly printed in this branch
  • Source inspected to do the same

Screenshots (if appropriate):

Screen Shot 2021-05-05 at 10 13 53 AM

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@jblz jblz added this to the 2.5.0 milestone May 5, 2021
@jblz jblz requested a review from a team May 5, 2021 14:14
@jblz jblz self-assigned this May 5, 2021
@GaryJones
Copy link
Contributor

As a side note, I would like to find a way to make these optional - that is, to have a filter that can conditionally add these integrations in, since they are redundant for the majority of sites that are not behind Cloudflare.

@jblz jblz merged commit 154c2e6 into develop May 6, 2021
@jblz jblz deleted the api-init-script-no-cfasync branch May 6, 2021 12:44
@jblz
Copy link
Contributor Author

jblz commented May 6, 2021

Agreed. Tracked in #293

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants