-
Notifications
You must be signed in to change notification settings - Fork 280
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
fix: gracefully handle invalid JSON in data-style attribute (#1393) #1394
Conversation
Thanks @georgel-pop-lr. Can you please run
|
Thanks @wincent, hope is fine now. |
src/plugins/embedurl.js
Outdated
} catch (e) { | ||
console.warn( | ||
'Unable to parse data-styles attribute!' | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple things here:
- Linter may warn about unused variable
e
here, so either use it (in the log statement) or mark it as intentionally unused by starting it with an underscore (ie._error
). Note also that we try to avoid abbreviations if we can (ie. prefererror
overe
), except for things likei
for loop variables. - We generally try to avoid anything that might resemble console spew in Liferay projects because it can be hard for customers to distinguish between bugs and mere diagnostic information (see guidelines on this); sometimes they see console output as a sign of bugginess and any "helpful" output we might include actually just confuses them. And they may be alarmed to see a message ending in an exclamation point. As such, I'd actually just swallow this error rather than log it — in some browsers, the stack trace that comes with
console.warn
looks particularly scary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lg2m
Is fixing #1393 If data-style attribute is not a valid JSON an exception will be thrown and the code will stop working.