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.

Sign up for GitHub

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

Check if window is available before using it #104

Merged
merged 4 commits into from
May 11, 2017
Merged

Check if window is available before using it #104

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d11e56c
Check if window is defined before using it
May 11, 2017
1b58323
Cleanup
May 11, 2017
ee69f0d
Make jshint a bit more relaxed
pimterry May 11, 2017
b9d9e00
Fix bug where window test was flipped
pimterry May 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 29 additions & 25 deletions lib/loglevel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,36 +101,40 @@
function persistLevelIfPossible(levelNum) {
var levelName = (logMethods[levelNum] || 'silent').toUpperCase();

// Use localStorage if available
try {
window.localStorage[storageKey] = levelName;
return;
} catch (ignore) {}

// Use session cookie as fallback
try {
window.document.cookie =
encodeURIComponent(storageKey) + "=" + levelName + ";";
} catch (ignore) {}
if (typeof window !== undefinedType) {
// Use localStorage if available
try {
window.localStorage[storageKey] = levelName;
return;
} catch (ignore) {}

// Use session cookie as fallback
try {
window.document.cookie =
encodeURIComponent(storageKey) + "=" + levelName + ";";
} catch (ignore) {}
}
Copy link
Owner

Choose a reason for hiding this comment

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

Sure, this is reasonable. I'd rather flip it and add a guard clause though, rather than indenting the whole function. Can you convert both of these into something like the below instead?

if (typeof window === undefinedType) return;

[...rest of the function...]

Copy link
Author

Choose a reason for hiding this comment

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

Sure

}

function getPersistedLevel() {
var storedLevel;

try {
storedLevel = window.localStorage[storageKey];
} catch (ignore) {}

// Fallback to cookies if local storage gives us nothing
if (typeof storedLevel === undefinedType) {
try {
var cookie = window.document.cookie;
var location = cookie.indexOf(
encodeURIComponent(storageKey) + "=");
if (location) {
storedLevel = /^([^;]+)/.exec(cookie.slice(location))[1];
}
} catch (ignore) {}
if (typeof window !== undefinedType) {
try {
storedLevel = window.localStorage[storageKey];
} catch (ignore) {}

// Fallback to cookies if local storage gives us nothing
if (typeof storedLevel === undefinedType) {
try {
var cookie = window.document.cookie;
var location = cookie.indexOf(
encodeURIComponent(storageKey) + "=");
if (location) {
storedLevel = /^([^;]+)/.exec(cookie.slice(location))[1];
}
} catch (ignore) {}
}
}

// If the stored level is not valid, treat it as if nothing was stored.
Expand Down