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

Add more metadata to loadScript errors #399

Merged
merged 4 commits into from
Feb 2, 2023

Conversation

arelra
Copy link
Member

@arelra arelra commented Feb 2, 2023

What are you changing?

Small update to #391 to add more metadata to the error messages and to help distinguish them from each other.

I've seen that targetSrc can be undefined as I assume ad-blockers are removing them from the dom. However we still have reference to src from the closure so report that in all cases.

@arelra arelra requested a review from a team as a code owner February 2, 2023 13:46
@changeset-bot
Copy link

changeset-bot bot commented Feb 2, 2023

🦋 Changeset detected

Latest commit: bf1dc86

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@guardian/libs Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added @guardian/libs 📦 npm Affects a @guardian package on NPM labels Feb 2, 2023
mxdvl
mxdvl previously approved these changes Feb 2, 2023
Copy link
Contributor

@mxdvl mxdvl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn’t this a minor as it’s a new feature?

@arelra arelra requested a review from mxdvl February 2, 2023 15:14
@arelra arelra merged commit 02f5817 into main Feb 2, 2023
@arelra arelra deleted the ravi/loadscript-add-metadata-to-error branch February 2, 2023 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@guardian/libs 📦 npm Affects a @guardian package on NPM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants