Extract Sign in With Google button JS to a module.

[JakePT]

Summary

Addresses issue:

• Extract Sign in with Google button JS to basic module #11950

Relevant technical choices

I've added @types/google.accounts as a dependency for use in the module.

PR Author Checklist

• My code is tested and passes existing unit tests.
• My code has an appropriate set of unit tests which all pass.
• My code is backward-compatible with WordPress 5.2 and PHP 7.4.
• My code follows the WordPress coding standards.
• My code has proper inline documentation.
• I have added a QA Brief on the issue linked above.
• I have signed the Contributor License Agreement (see https://cla.developers.google.com/).

---

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

Code Reviewer Checklist

• Run the code.
• Ensure the acceptance criteria are satisfied.
• Reassess the implementation with the IB.
• Ensure no unrelated changes are included.
• Ensure CI checks pass.
• Check Storybook where applicable.
• Ensure there is a QA Brief.
• Ensure there are no unexpected significant changes to file sizes.

Merge Reviewer Checklist

• Ensure the PR has the correct target branch.
• Double-check that the PR is okay to be merged.
• Ensure the corresponding issue has a ZenHub release assigned.
• Add a changelog message to the issue.

[github-actions]

🤖 This comment is automatically updated by CI workflows. Each section is managed independently.

🎭 Playwright reports for 7b735dc:

• ✅ WordPress 5.2.21
• ✅ WordPress latest
• ✅ WordPress nightly

📚 Storybook for 7b735dc:

• View Storybook

📦 Build files for 7b735dc:

• Download development build
• Download production build

[gemini-code-assist]

Code Review

This pull request refactors the Sign in with Google integration by migrating the inline frontend JavaScript from PHP into a dedicated TypeScript bundle, passing configuration data via a global window object. The review feedback highlights critical issues in the new TypeScript implementation, specifically a regression where missing dataset attributes can overwrite default button options with undefined, and potential ReferenceErrors when referencing the global google object if the external library fails to load. Implementing the suggested safety checks and fallback values will ensure robust execution.

[gemini-code-assist]

The google global variable is loaded asynchronously via an external script. If the script fails to load or is blocked by privacy extensions/ad-blockers, referencing google directly will throw a ReferenceError at runtime, even with optional chaining (google?.). To prevent script crashes, wrap the initialization in a check to ensure google is defined and has the expected properties.

	if ( typeof google !== 'undefined' && google?.accounts?.id ) {
		google.accounts.id.initialize( {
			client_id: data.clientID, // eslint-disable-line camelcase
			callback: ( response ) => handleCredentialResponse( response, data ),
			library_name: 'Site-Kit', // eslint-disable-line camelcase
		} as google.accounts.id.IdConfiguration ); // eslint-disable-line sitekit/acronym-case
	}

[JakePT]

Updated to check existence of google.accounts.id early and return if it doesn't exist.

[gemini-code-assist]

Ensure that the google global variable is defined before calling prompt() to avoid a ReferenceError if the external script fails to load.

Suggested change

	if ( data.shouldShowOneTapPrompt ) {
	google?.accounts.id.prompt();
	}
	if ( data.shouldShowOneTapPrompt && typeof google !== 'undefined' && google?.accounts?.id ) {
	google.accounts.id.prompt();
	}

[JakePT]

Updated to check existence of google.accounts.id early and return if it doesn't exist.

[aaemnnosttv]

Thanks @JakePT – this is off to a good start. I have a few suggestions for how we can improve this a bit more structurally which should also simplify a bit of the implementation / testing too.

[aaemnnosttv]

This may have been renamed unintentionally but the server side expects this to be using the name as before (which shouldn't change here). See

site-kit-wp/includes/Modules/Sign_In_With_Google/Existing_User_Authenticator.php

Lines 72 to 74 in 546a44f

	// Tie the link request to the current WordPress session so the
	// user's auth cookie alone can't validate a cross-site request.
	$nonce = $input->filter( INPUT_POST, 'connect_nonce' );

Suggested change

	body.append( 'connectNonce', data.connectNonce );
	body.append( 'connect_nonce', data.connectNonce );

[aaemnnosttv]

This is mostly what I had in mind for this entry point – you can see an example of this in assets/js/analytics-advanced-tracking.ts which is worth aligning a bit more closely with.

One important difference is the use of the global variable which we don't really need. We've solved this differently more recently in our inline Pointers JS which uses data attributes (set by the WP inline script function) to pass data to the script without needing to write JS dynamically with PHP.

Aside from that, the general ideas are to keep the function a bit more pure and typed, so that we can handle things like the availability of google.accounts.id before we call it, and the types and tests+mocks become simpler as a result. The function itself only actually needs google.accounts.id so we can pass that in when called, and only call it when it's there.

Elaborated details 🤖

Structure suggestion — lean further on the analytics-advanced-tracking pattern (non-blocking)

This looks modeled on analytics-advanced-tracking already (same window._googlesitekit… global), which is a good base — but that sibling predates both TS and the idea of passing config via script-tag attributes, so I think we can take it a step further:

Split it like the sibling does:

assets/js/sign-in-with-google.ts                            ← thin entrypoint/bootstrap
assets/js/sign-in-with-google/set-up-sign-in-with-google.ts ← setUpSignInWithGoogle + handleCredentialResponse
assets/js/sign-in-with-google/set-up-sign-in-with-google.test.ts
assets/js/sign-in-with-google/types.ts                      ← SignInWithGoogleData

Drop the global, carry config on the script tag instead. Hang it off the bundle's own <script> tag as a data- attribute and read it back with document.currentScript.dataset (this is what Pointers::print_pointer_script() does). Bonus: wp_print_script_tag() escapes attribute values for us automatically. Whether the config travels as one JSON blob or separate data- attributes is your call — both get the escaping. (If you split it: dataset returns everything as strings, so the booleans and the TTL number need converting and defaultButtonOptions becomes its own JSON attribute anyway; a single blob avoids that. Fine either way.) Once the global's gone, the SignInWithGoogleData import in globals.d.ts drops out too.

Have the entrypoint do the messy parts, and pass dependencies into the function. The sibling already does this — it hands sendEvent into setUpAdvancedTracking rather than reaching for window.gtag. Same idea here:

• Entrypoint: read config off dataset, check google?.accounts?.id is actually loaded, then call setUpSignInWithGoogle( google.accounts.id, config ).
• Function: takes the config and google.accounts.id as arguments, never touches a global. Because it receives google.accounts.id already verified, the internal typeof google === 'undefined' guard goes away — which is also the cleanest answer to the Gemini note about checking things are defined before calling methods (it happens once, up front).

Why it's worth it: the test currently has to do ( window.google as unknown as { … } ) = { … } to fake the global. With the function taking plain arguments, the test just passes a config object and a few jest.fn()s — no global juggling, no await import(). So this mostly removes existing awkwardness rather than adding ceremony.

Two notes to keep it from sprawling: keep handleCredentialResponse in the same file as setUpSignInWithGoogle (only used there — the useful seam is entrypoint vs. logic, not function vs. function), and the only firm asks are (1) the function takes a typed config object and (2) it takes google.accounts.id as an argument with the load check moved to the bootstrap.