Provided we have an element with the following conditions:
has no text content
has a non-string value (e.g. number)
Then clarity throws an error when interacting with that element
Error
The following error is thrown and logged:
TypeError: n.replace is not a function
at fn (clarity.js:2:26237)
at dn (clarity.js:2:26080)
at HTMLDocument.<anonymous> (clarity.js:2:56206)
at HTMLDocument.sentryWrapped (helpers.ts:100:1)
function text(element: Node): string {
let output = null;
if (element) {
// Grab text using "textContent" for most HTMLElements, however, use "value" for HTMLInputElements and "alt" for HTMLImageElement.
let t = element.textContent || (element as HTMLInputElement).value || (element as HTMLImageElement).alt;
if (t) {
// Replace multiple occurrence of space characters with a single white space
// Also, trim any spaces at the beginning or at the end of string
// Finally, send only first few characters as specified by the Setting
output = t.replace(/\s+/g, Constant.Space).trim().substr(0, Setting.ClickText);
}
}
return output;
}
Where t is the element, in my case n ends up having a value of 2 (number, not "2" as string), which is the value of the value property:
Proposed solution
t.value is assumed to be a string and used as such by subsequent functions, but that it not the case. It must be converted to string before using functions like replace. Some libraries and systems set typed values on input, input-like or entirely non-input fields which ends up causing errors because of mismatched types.
I understand it's not necessarily Clarity's "problem" per se - I am not sure how the web standards suggest to handle such a case, or if setting non-string values on elements should actually be possible or not. But I believe that with the nature of the web having many unclear cases, and a long history of not-amazing code, Clarity should be able to handle such types of data, so that it can collect the information properly from a wider collection of projects.
Description of change in this PR
In this PR, I updated the code to be able to handle non-string values in the value prop of an element.
The value is wrapped with String() call to convert it to string in an easy way. If the value would be null or undefined, it would cause undesired behavior in the logic. So to keep the existing logic unbroken, it would fall back to empty string if missing, which then in turn will trigger the short circuiting as normal. So the change should only fix an edge-case without breaking existing behavior.
Short description of problem
Provided we have an element with the following conditions:
Error
The following error is thrown and logged:
This points to the following code (link:
Where
t
is the element, in my casen
ends up having a value of2
(number, not"2"
as string), which is the value of thevalue
property:Proposed solution
t.value
is assumed to be a string and used as such by subsequent functions, but that it not the case. It must be converted to string before using functions likereplace
. Some libraries and systems set typed values on input, input-like or entirely non-input fields which ends up causing errors because of mismatched types.I understand it's not necessarily Clarity's "problem" per se - I am not sure how the web standards suggest to handle such a case, or if setting non-string values on elements should actually be possible or not. But I believe that with the nature of the web having many unclear cases, and a long history of not-amazing code, Clarity should be able to handle such types of data, so that it can collect the information properly from a wider collection of projects.
Description of change in this PR
In this PR, I updated the code to be able to handle non-string values in the
value
prop of an element.The value is wrapped with
String()
call to convert it to string in an easy way. If the value would benull
orundefined
, it would cause undesired behavior in the logic. So to keep the existing logic unbroken, it would fall back to empty string if missing, which then in turn will trigger the short circuiting as normal. So the change should only fix an edge-case without breaking existing behavior.