✨Analytics: CookieWriter #18406
✨Analytics: CookieWriter #18406
Conversation
|
|
||
| if (isInFie(this.element_)) { | ||
| // TODO: Need the consider the case for shadow doc. | ||
| // QQ: Is this even an error since vendor defines it? |
There was a problem hiding this comment.
right, let's not print an error
| this.readyPromise_.resolve(); | ||
| return; | ||
| } | ||
| if (isProxyOrigin(this.win_.location)) { |
There was a problem hiding this comment.
nit: let's move cases that we ignore to the beginning (together with FIE).
| const ids = Object.keys(inputConfig); | ||
| for (let i = 0; i < ids.length; i++) { | ||
| const cookieId = ids[i]; | ||
| const cookieStr = inputConfig[cookieId]; |
There was a problem hiding this comment.
let's call them cookieName & cookieValue
| const cookieStr = inputConfig[cookieId]; | ||
| if (typeof cookieStr === 'string') { | ||
| this.promises_.push(this.expandAndWrite_(cookieId, cookieStr)); | ||
| } |
| } | ||
| } | ||
|
|
||
| Promise.all(this.promises_).then(() => { |
There was a problem hiding this comment.
nit: this.readyPromise_.resolve(Promise.all(this.promises_))
| /** @private {!../../../src/utils/promise.Deferred} */ | ||
| this.readyPromise_ = new Deferred(); | ||
|
|
||
| this.init_(config); |
There was a problem hiding this comment.
instead of doing all the work in constructor, we can move it to whenReady. Actually, we'd rename whenReady to write or run
run() {
...
return readyPromise;
}
There was a problem hiding this comment.
Good idea. I'll switch to write, but one thing I want to make sure is that we only write cookies once no matter how many times the write() has been invoked. To achieve that I'll need to make some extra change.
For example can longer switch to this.readyPromise_.resolve(Promise.all(this.promises_))
|
One design change: Instead, for now, cookie write will check the string, and make sure that only string like |
|
@lannka Please take another look. I added logic to apply stricter rule to the cookieValue format. |
* init commit * linter * address comment * apply stricter rule * disable inabox
* init commit * linter * address comment * apply stricter rule * disable inabox
The PR introduces the change to read the
writeCookiesconfig. Expand the value and write to cookie.Decision highlights.
<amp-analytics>'s initializationQUERY_PARAMandLINKER_PARAM(to be added in separate PR)cc @calebcordry