Check if hashUrlSearchParams has p[meta_page_view_id] #6710
Conversation
|
Size Change: +20 B (0%) Total Size: 2.37 MB ℹ️ View Unchanged
|
| @@ -124,6 +120,10 @@ export function Events({ geoId }: Props) { | |||
| hashUrlSearchParams.toString(), | |||
| )}`; | |||
|
|
|||
| if (!hashUrlSearchParams.has('p[meta_page_view_id]')) { | |||
There was a problem hiding this comment.
Not sure if we'd want to use it here, but apparently .has takes an optional second arg which represents the expected value. So we could use that to verify not only that the key is set, but also that it has the value we want:
if (!hashUrlSearchParams.has('p[meta_page_view_id]', pageViewId)) {
https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams/has
There was a problem hiding this comment.
Nice idea! I'll do this 👍
There was a problem hiding this comment.
I've added this 2nd parameter to my has() call but the type checking is now returning an error: Expected 1 arguments, but got 2, very odd as the type definition for has() includes an optional 2nd value parameter.
There was a problem hiding this comment.
This discussion highlights the same issue: microsoft/TypeScript#55569, the conclusion is has() is not adequately supported, in this situation the 'algorithm' can decide that a feature is not well supported by browsers and thus removes it: https://github.com/microsoft/TypeScript-DOM-lib-generator#when-the-type-is-missing. However the article suggests this happens if a feature is not supported by two or more browser engines, however the MDB for has() shows it is supported in all browsers latest versions: https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams/has so I wouldn't expect the algorithm to flag it as unsupported.
A temporary workaround could be to this to our any.d.ts file:
global {
interface URLSearchParams {
has(name: string, value?: any): boolean;
}
}
@tjmw what do you think?
There was a problem hiding this comment.
Weird! Yeah for me VSCode is happy, it looks like its using it's own dom types, but running yarn validate I'm seeing the same error. I can't see anything in our tsconfig which would determine the dom types used, maybe it's something we're getting from @guardian/tsconfig?
There was a problem hiding this comment.
Weird! Yeah for me VSCode is happy, it looks like its using it's own dom types, but running
yarn validateI'm seeing the same error. I can't see anything in our tsconfig which would determine the dom types used, maybe it's something we're getting from@guardian/tsconfig?
Yeah I wondered whether there's a compatibility issue between our minimum supported browsers and the feature, I couldn't find where in the Typescript config we define minimum supported browser versions. I'll check @guardian/tsconfig now!
There was a problem hiding this comment.
Nothing in support-frontend/tsconfig.json the @guardian/tsconfig to indicate minumum browser versions supported.
There was a problem hiding this comment.
I seem to remember reading that the lib types are somehow derived from the target if not specified explicitly, which in our case is esnext (coming from @guardian/tsconfig). Esnext means the laces ecma script version supported by the version of TypeScript you're on, which for us is 5.1.6. I think the latest target supported by that version is ES2020 but I'm finding it hard to prove that.
|
Seen on PROD (merged by @GHaberis 8 minutes and 55 seconds ago)
Sentry Release: support-client-side, support |
What are you doing in this PR?
An alternate log to #6694, as the original log didn't report anything in ~5 days. I wondered whether
hashUrlSearchParamscontained a'p[meta_page_view_id]key as we'd expect, we can check this using https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams/has. If not log an expection in Sentry.