Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

attribution property can be undefined #264

Closed
ArgonAlex opened this issue Sep 29, 2022 · 3 comments · Fixed by #265
Closed

attribution property can be undefined #264

ArgonAlex opened this issue Sep 29, 2022 · 3 comments · Fixed by #265

Comments

@ArgonAlex
Copy link

Using the attribution build, I was seeing Uncaught TypeError: Cannot read property 'largestShiftTarget' of undefined errors in our client logs. Unfortunately this means we missed tracking some CLS events until we could deploy a fix with a truthy check for attribution.

Our usage is very similar to the documentation, which will also run into this error because there is no truthy check for attribution.

It seems that either:
a) The TS typings and documentation should be updated to take into account attribution possibly being undefined
or
b) The code should be updated so attribution is always at least an empty object

@philipwalton
Copy link
Member

The intention is that the attribution property is always present when using the attribution build, so the documentation is correct and if you're finding cases where that's not happening then it may be a bug.

Looking at the code for onCLS() now, it does seem like it could be possible if the user were on a browser that supports CLS but doesn't support the LayoutShiftAttribution interface, which looks like was true for Chrome version 77-83.

I will update the code to ensure there's always an attribution property, but to make sure nothing else is going on, are you able to see what browser and version the users reporting this error had?

@ArgonAlex
Copy link
Author

Thanks for looking into this!

Our logs are from Sept 15 (when we started using the attribution build) to Sept 28 (when our fix was deployed). From those logs I'm seeing almost every Chrome version >= 62 across all of the major platforms, Android Chrome 105 being the most common (which makes sense since it was the generally the most common Chrome version for our users as of these log dates). So it seems to be more than just Chrome 77-83.

@philipwalton
Copy link
Member

This should be fixed in version 3.0.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants