Better Regex to ignore $this variables in the code

[jesse-deboer]

At this moment the regex matches with the word this but also with $this, that should not be the case. This way $this is ignored from being replaced, this is used in some 3rd party scripts.

[vercel[bot]]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/builder-io/partytown/CHe6saxhRKV5qro47RApC2WZFEJ6
✅ Preview: https://partytown-git-fork-jesse-deboer-patch-1-builder-io.vercel.app

[adamdbradley]

Doesn't look like it's passing webkit. @jesse-deboer would you be able to see what the issue is? Thanks

[jesse-deboer]

Doesn't look like it's passing webkit. @jesse-deboer would you be able to see what the issue is? Thanks

Hi @adamdbradley, this was already broken as you reported in this issue https://github.com/BuilderIO/partytown/issues/131.

I will try to find out why it's not working, but it's not because of this PR I think ..

[adamdbradley]

It looks like this error is only for webkit that every test fails, which is different from the other issue (i think).

[adamdbradley]

Sorry but this doesn't work on Safari. When I run it locally Safari will error with SyntaxError: Invalid regular expression: invalid group specifier name.

[jesse-deboer]

@adamdbradley you are indeed right, Safari unfortunately still doesn't support look-behind. I'll try and make this RegEx so that it will also support Safari!

[nilesh-maurya]

@jesse-deboer can you test below replace function, i think it should work for Safari also.

scriptContent.replace(/\bthis\b/g, (match, offset, originalStr) =>  (offset > 0 && originalStr[offset - 1] !== "$") ? '(thi$(this)?window:this)' : match);

Explanation for above code

Link: MDN docs for String::replace

Instead of using Look-behind to check for $, let’s do manual checking based on match substring.

first let’s revert to previous regex.

// look behind regex
scriptContent.replace(/(?<!\$)\bthis\b/g, '(thi$(this)?window:this)').replace(/\/\/# so/g, '//Xso')

// previous regex
scriptContent.replace(/\bthis\b/g, '(thi$(this)?window:this)').replace(/\/\/# so/g, '//Xso')

// more specifically first replace function
scriptContent.replace(/\bthis\b/g, '(thi$(this)?window:this)')

Now this regex will match all this (ie. this and $this). As per MDN docs second argument of string::replace function can be either string or function.

same code as above with second argument as function

// return value will be used as replacement value
scriptContent.replace(/\bthis\b/g, () => {
	return '(thi$(this)?window:this)';
})

The replacement function accepts few arguments. check the docs for more info about arguments.

function replacer(match, p1, p2, /* …, */ pN, offset, string, groups) {
  return replacement;
}

look at below example, offset will always gives us the index of matched substring.

so using offset we can check if previous index is “$” or not (i.e. offset - 1).

// there are no capture groups, so no need to accept p1,...pn args
scriptContent.replace(/\bthis\b/g, (match, offset, originalStr) => {
	if (offset > 0 && originalStr[offset - 1] !== "$") {
		return '(thi$(this)?window:this)';
	}
})

As per our regex we are matching all the substring with “this” and inside the function we are only returning the replacement value if we don’t find the “$” before matched substring but all other time we are returning “undefined”. To take care of this we’ll just have to return matched substring as it is.

scriptContent.replace(/\bthis\b/g, (match, offset, originalStr) => {
	if (offset > 0 && originalStr[offset - 1] !== "$") {
		return '(thi$(this)?window:this)';
	}
// otherwise just return match character.
return match;
})

// one liner with ternary operator
scriptContent.replace(/\bthis\b/g, (match, offset, originalStr) =>  (offset > 0 && originalStr[offset - 1] !== "$") ? '(thi$(this)?window:this)' : match);

[vercel[bot]]

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

Name	Status	Preview	Comments	Updated
partytown	✅ Ready (Inspect)	Visit Preview	💬 Add your feedback	Mar 21, 2023 at 3:46PM (UTC)

[jesse-deboer]

@jesse-deboer can you test below replace function, i think it should work for Safari also.

scriptContent.replace(/\bthis\b/g, (match, offset, originalStr) =>  (offset > 0 && originalStr[offset - 1] !== "$") ? '(thi$(this)?window:this)' : match);

Explanation for above code

Great solution, let's see if it works, just committed the code to the patch!

[peterjaap]

@adamdbradley what's the status on this?