fix: gtm scripts not working with loadScriptsOnMainThread by id

[Cutch]

GTM uses the text field when settings up an element.

Ex. from the gtm script

Added field watch for text to the element patch code. Test code is added is well.

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
partytown	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 13, 2023 4:12pm

[adamdbradley]

This looks great, but should the test be setting text? Looks like there's two tests now setting innerHTML: https://github.com/BuilderIO/partytown/pull/399/files#diff-81c57e5d6a7ba968191267a24106e9e317d39bacbee22836297fa4814eca49c4R157

[Cutch]

@adamdbradley actually the test is setting text via script.text. The set by innerHTML is just setting the response result to testInlineTextScript which is only used by the test suite to assert the result, which is already outside of partytown at that point.

[adamdbradley]

Sorry I must be misunderstanding then. How is this testing that setting text is working? Can help point out where the test would have failed before this update? Thanks

[Cutch]

ya no problem, note where it says script.text that's the point where it would have failed previously, because partytown was not watching for .text. I added some comments to further explain:

<code id="testInlineTextScript"></code>
<script type="text/javascript">const globalVariable2 = 12;</script>
<script type="text/partytown">
  (function () {
    // This script is now running from partytown
	
	// Create a new script to run outside of partytown
	const script = document.createElement('script');

	script.type = "text/javascript";
	script.id = "inline-text-test-script";

	// Set the body of the script via script.text
	// This is the main logic of the function, which previously did not work with partytown
	script.text = `
	  (function () {`+
	    // This script is only for the test suite, and is run outside of partytown therefore it can be ignored
	    // The test suite is just going to watch and see if testInlineTextScript is set to 12
            // which would not happen if this was running in the webworker still as globalVariable2 is only set in the main thread
		`const testEl = document.getElementById('testInlineTextScript');
		testEl.className = 'testInlineTextScript';
		testEl.innerHTML = globalVariable2;
	  })();
	`;
	document.body.appendChild(script);
  })();
</script>

[Cutch]

@adamdbradley Hey, I just wanted to follow up to see if you need any more information/if the PR is good to go

[vladetsky]

@adamdbradley I have the same bug as well, and this PR fixes it. You can repro it by creating Custom HTML tag in GTM, add there something like <script type="text/javascript" id="some-id"></script> and add some-id to loadScriptsOnMainThread. You'll see that injected script still loads in partytown worker.

[slawekkolodziej]

For what it's worth, I suggested fix for the the same issue some time ago here: https://github.com/BuilderIO/partytown/pull/234, I believe the MR was mistakenly closed. Now I don't quite remember why, but my proposed fix had some additional logic in the innerHTML getter.

[gioboa]

This PR is old, so I'm closing this for now. Feel free to re-open it if it's still valid. Thanks.