Pull Scripts out of PHP & Begin removing jQuery dependency #236

jblz · 2021-04-19T14:12:22Z

This is the first pass at pulling the plugin JavaScript out of PHP files and into enqueued assets.

The widget script is not included in this change as this PR was already getting pretty large. In addition, rewriting it with vanilla JavaScript is not a trivial effort and we're considering re-working it anyhow.

As such, this does not fully complete #134 or #158, but it's at least a step in that direction.

Additionally, we're able to use the WordPress i18n tooling to wrap a string in the Admin page JS for translation (see 3c62c8d). Again, it doesn't fully satisfy #160, but it paves the way for doing so.

To Test

Automated Tests

changes should be correct and comprehensive
Automated tests should pass

Building the Assets

Follow instructions to build the included assets
Make sure the instructions are clear and easy to follow

Test the plugin

The tracking pixel should be correctly enqueued
The API initialization script should be correctly enqueued
The Admin JS should be correctly enqueued
The Admin CSS should be correctly applied
The Recommended Content widget loading should be unaffected
Recommended content should be appropriately displayed
In general, there should be no changes that are noticeable to end users by virtue of this change (unless they open their dev tools panel 😄)

jblz · 2021-04-22T17:59:13Z

src/lib/init-api/index.js

+function uuidProfileCall( { apikey, uuid } ) {
+	const profileScript = document.createElement( 'script' );
+	profileScript.src = `https://api.parsely.com/v2/profile?apikey=${ encodeURIComponent(
+		apikey
+	) }&uuid=${ uuid || '' }&url=${ window.location.href }&callback=&_=${ +new Date() }`;
+	document.querySelector( 'head' ).appendChild( profileScript );
+}


This is the part I'm least confident about. I'm attempting to replace this "jsonp" call:

wp-parsely/parsely-javascript.php

Lines 24 to 32 in 14c342a

function uuidProfileCall() {

var rootUrl = 'https://api.parsely.com/v2/profile?apikey=<?php echo esc_html( $parsely_options['apikey'] ); ?>';

var uuid = '&uuid=' + PARSELY.config.parsely_site_uuid;

var requestUrl = rootUrl + uuid + '&url=' + window.location.href;

jQuery.ajax({

url: requestUrl,

dataType: "jsonp"

});

}

...which isn't obvious to me how it's used or what it does. @hbbtstar -- can you shed any light on this?

As per the internal P2 post, CORS support in browsers is now enough, that JSONP isn't needed any more.

GaryJones

I love where this is going!

Added a few comments, including 3-4 items that we may want to spin up into separate issues/PRs, if you don't want to address them in this one.

build/admin-page.asset.php

class-parsely-recommended-widget.php

package.json

src/lib/admin-page/index.js

tests/all-test.php

wp-parsely.php

GaryJones · 2021-04-23T09:46:27Z

wp-parsely.php

+		}
+
+		if ( $handle === 'wp-parsely-tracker' ) {
+			$tag = preg_replace( '/ id=(\"|\')wp-parsely-tracker-js\1/', ' id="parsely-cfg"', $tag );


Do we know why this change of ID is needed?

I really don't know. This is another case of trying to not break anything without understanding why it is the way it is.

@hbbtstar -- any thoughts as to the necessity of having particular id attributes to the script tags?

…ode w/ wp functions

* pull PARSELY_PLUGIN_URL into a constant & ref in the test * cover multisite as well * drop the $parsely_html file since scripts are no longer printed directly

tests/class-testutils.php

…instead-of-jsonp Convert Profile API call to simple XHR - no more JSONP

…query-dependency

…s now

src/class-parsely.php

src/js/lib/admin-page/index.js

jblz self-assigned this Apr 19, 2021

jblz mentioned this pull request Apr 19, 2021

Try: React Components in the Admin Script #239

Closed

jblz force-pushed the remove/jquery-dependency branch 4 times, most recently from 08d7f75 to 22f6e43 Compare April 22, 2021 17:57

jblz requested a review from a team April 22, 2021 18:02

jblz added this to the 2.5.0 milestone Apr 22, 2021

jblz marked this pull request as ready for review April 22, 2021 18:02

jblz added the Status: Needs Review label Apr 22, 2021

jblz commented Apr 22, 2021

View reviewed changes

jblz requested a review from jase-d-ace April 22, 2021 18:20

jblz changed the title ~~WIP: Pull Scripts out of PHP & Begin removing jQuery dependency~~ Pull Scripts out of PHP & Begin removing jQuery dependency Apr 22, 2021

jblz force-pushed the remove/jquery-dependency branch from e2e260c to 68c6bf9 Compare April 22, 2021 19:32

GaryJones requested changes Apr 23, 2021

View reviewed changes

jblz added 15 commits April 23, 2021 15:31

append a script tag instead of jQuery jsonp

0259ff5

rm ensure_jquery_exists function that enqueues jquery

06bdce3

write admin recrawl notices w/ vanilla js

eed7f6b

Pull API initialization into a js file. Enqueue it and the tracking c…

df02cb5

…ode w/ wp functions

Pull script_loader_tag action into method

2a2ae6c

Split out API initialization and only source when secret is set

a896c0b

build & enqueue the admin page js

6bc3494

limit settings page script to the parsely settings page

40de6b1

Improve script handles, only call wp_localize_script if loading api

91cc4ad

functions for asset cache buster & identifying test env

8b7d532

Tests for script enqueues and prints, cache buster, & plugin version

6a5c9bc

* pull PARSELY_PLUGIN_URL into a constant & ref in the test * cover multisite as well * drop the $parsely_html file since scripts are no longer printed directly

Use wp.domReady

1a25340

Wrap the string in the admin js for translation

cf751ae

include built assets

726851b

gitignore source map files used in dev

d4b3c77

jblz added 4 commits April 30, 2021 18:06

Define PARSELY_PLUGIN_DIR & use it in includes

f1146cd

fixup settings include

1528655

New Test Helper Parsely\Tests\TestUtils::set_options

228867f

Move DEFAULT_OPTIONS to a class constant

29dd096

GaryJones requested changes May 1, 2021

View reviewed changes

tests/class-testutils.php Outdated Show resolved Hide resolved

jblz added 3 commits May 3, 2021 08:03

Move the set_options helper to the existing TestCase class

8739ab0

Convert Profile API call to simple XHR - no more JSONP

55cb973

build assets

0f800c5

jblz mentioned this pull request May 3, 2021

Convert Profile API call to simple XHR - no more JSONP #273

Merged

jblz added 6 commits May 3, 2021 09:59

Merge pull request #273 from Parsely/remove/jquery-dependency--fetch-…

55f4b2a

…instead-of-jsonp Convert Profile API call to simple XHR - no more JSONP

Merge branch 'develop' of github.com:Parsely/wp-parsely into remove/j…

1932470

…query-dependency

Fix Translation Wrapping of the Recrawl Notice

91e2657

include updated built assets

60978c3

Remove src/parsely-parsely-page.php -- it's handled in the views file…

e3fa059

…s now

Prevent inclusion of package-lock from the built plugin

95dd004

GaryJones requested changes May 3, 2021

View reviewed changes

src/class-parsely.php Outdated Show resolved Hide resolved

src/js/lib/admin-page/index.js Outdated Show resolved Hide resolved

GaryJones added 2 commits May 3, 2021 19:40

JS: Follow standard translator comment format

7dada4d

Filter: Add since tag

a704a74

GaryJones approved these changes May 3, 2021

View reviewed changes

jblz merged commit 74725db into develop May 3, 2021

jblz deleted the remove/jquery-dependency branch May 3, 2021 18:50

acicovic removed the Status: Needs Review label Apr 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pull Scripts out of PHP & Begin removing jQuery dependency #236

Pull Scripts out of PHP & Begin removing jQuery dependency #236

jblz commented Apr 19, 2021 •

edited

jblz Apr 22, 2021

GaryJones Apr 28, 2021

GaryJones left a comment

GaryJones Apr 23, 2021

jblz Apr 23, 2021

	function uuidProfileCall() {
	var rootUrl = 'https://api.parsely.com/v2/profile?apikey=<?php echo esc_html( $parsely_options['apikey'] ); ?>';
	var uuid = '&uuid=' + PARSELY.config.parsely_site_uuid;
	var requestUrl = rootUrl + uuid + '&url=' + window.location.href;
	jQuery.ajax({
	url: requestUrl,
	dataType: "jsonp"
	});
	}

Pull Scripts out of PHP & Begin removing jQuery dependency #236

Pull Scripts out of PHP & Begin removing jQuery dependency #236

Conversation

jblz commented Apr 19, 2021 • edited

To Test

Automated Tests

Building the Assets

Test the plugin

jblz Apr 22, 2021

Choose a reason for hiding this comment

GaryJones Apr 28, 2021

Choose a reason for hiding this comment

GaryJones left a comment

Choose a reason for hiding this comment

GaryJones Apr 23, 2021

Choose a reason for hiding this comment

jblz Apr 23, 2021

Choose a reason for hiding this comment

jblz commented Apr 19, 2021 •

edited