-
Notifications
You must be signed in to change notification settings - Fork 27.4k
fix(cookieReader): safely access $document so it can be mocked #11388
Conversation
I'm really not sure that this is the right way to solve this. I'm thinking it might be a good idea to have some mock implementation of |
@petebacondarwin probably comes up with a good idea how to solve this, he always does :) |
return lastCookies; | ||
}; | ||
} | ||
|
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.
I think this is probably overkill. If the developer mocks out window and doesn't include a document then lots of stuff is going to break. Instead, how about we simply do
lastCookieString = rawDocument.cookie || '';
In the function below.
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.
actually this is the only place that breaks, you can mock out $window
and not include $document
. See https://github.com/angular/angular.js/blob/master/test/ng/windowSpec.js
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.
I can also do something like this, if you prefer:
rawDocument = $document[0] || {};
lastCookieString = rawDocument.cookie || '';
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.
Ugh! Mocking window without a document is surely crazy juice!
OK let's go with this version:
rawDocument = $document[0] || {};
lastCookieString = rawDocument.cookie || '';
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.
okay, amended.
LGTM - feel free to merge |
Closes #11373