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

$cookies need callback once cookie value is set. #6411

Closed
manishtpatel opened this Issue Feb 23, 2014 · 16 comments

Comments

Projects
None yet
@manishtpatel

manishtpatel commented Feb 23, 2014

Right now if you set $cookies values then call $http immediately, cookie is not updated in the request. if $cookies returns before saving the cookie in the browser then we need callback to execute code that relies on cookie.

$cookies.myName = 'foo'

// request headers are missing myName cookie update from above line
$http.get('/api/getmystar')

To workaround this issue i have to use setTimeout to wait for some time before i can proceed.

It either need to not set the cookie asynchronously or provide a call back function, call back is not possible with $cookies but $cookieStore atleast should solve this issue.

summary

cookie writes are flushed asynchronously, which makes it hard to predict when the cookie will be picked by other browser apis like XMLHttpRequest used by $http

@bfricka

This comment has been minimized.

Show comment
Hide comment
@bfricka

bfricka Feb 27, 2014

Maybe what should be addressed is the fact that ngCookies works via polling instead via some other customary Angular pattern ($digest, promises, etc).

When I first saw the way the API was broken down in $cookies and $cookieStore, I thought $cookies was an object that just updated (i.e. did read/write to document.cookie) on $digest, and $cookieStore was essentially a direct wrapper around document.cookie. I.e. $cookieStore.put('key', 'value'); splits and serializes the cookie and adds your value.

When I looked at the actual implementation, I found it really surprising to find that it updates on a poll function. This is a really odd choice.

I think adding a callback to $cookieStore is poor idea, b/c it doesn't really jive w/ Angular well. But I do think this should be addressed (e.g. by returning a promise).

bfricka commented Feb 27, 2014

Maybe what should be addressed is the fact that ngCookies works via polling instead via some other customary Angular pattern ($digest, promises, etc).

When I first saw the way the API was broken down in $cookies and $cookieStore, I thought $cookies was an object that just updated (i.e. did read/write to document.cookie) on $digest, and $cookieStore was essentially a direct wrapper around document.cookie. I.e. $cookieStore.put('key', 'value'); splits and serializes the cookie and adds your value.

When I looked at the actual implementation, I found it really surprising to find that it updates on a poll function. This is a really odd choice.

I think adding a callback to $cookieStore is poor idea, b/c it doesn't really jive w/ Angular well. But I do think this should be addressed (e.g. by returning a promise).

@IgorMinar IgorMinar added the gh: issue label Mar 6, 2014

@IgorMinar IgorMinar added the type: bug label Jul 24, 2014

@IgorMinar IgorMinar added this to the Backlog milestone Jul 24, 2014

@btford btford removed the gh: issue label Aug 20, 2014

shahata added a commit to shahata/angular.js that referenced this issue Dec 19, 2014

refactor($cookieStore): use $browser directly instead of $cookies
This is important in order to avoid the current mechanism where polling is required to get the latest cookies and digest is required to flush the latest changes.

Closes #6411, #7631

shahata added a commit to shahata/angular.js that referenced this issue Dec 19, 2014

refactor($cookieStore): use $browser directly instead of $cookies
This is important in order to avoid the current mechanism where polling is required to get the latest cookies and digest is required to flush the latest changes.

Closes #6411, #7631

shahata added a commit to shahata/angular.js that referenced this issue Dec 19, 2014

refactor($cookieStore): use $browser directly instead of $cookies
This is important in order to avoid the current mechanism where polling is required to get the latest cookies and digest is required to flush the latest changes.

Closes #6411, #7631

shahata added a commit to shahata/angular.js that referenced this issue Jan 5, 2015

refactor($cookieStore): use $browser directly instead of $cookies
This is important in order to avoid the current mechanism where polling is required to get the latest cookies and digest is required to flush the latest changes.

Closes #6411
Closes #7631

shahata added a commit to shahata/angular.js that referenced this issue Feb 28, 2015

refactor($cookieStore): use $browser directly instead of $cookies
This is important in order to avoid the current mechanism where polling is required to get the latest cookies and digest is required to flush the latest changes.

Closes #6411
Closes #7631

shahata added a commit to shahata/angular.js that referenced this issue Feb 28, 2015

refactor($cookieStore): use $browser directly instead of $cookies
This is important in order to avoid the current mechanism where polling is required to get the latest cookies and digest is required to flush the latest changes.

Closes #6411
Closes #7631

shahata added a commit to shahata/angular.js that referenced this issue Feb 28, 2015

refactor($cookieStore): use $browser directly instead of $cookies
This is important in order to avoid the current mechanism where polling is required to get the latest cookies and digest is required to flush the latest changes.

Closes #6411
Closes #7631

shahata added a commit to shahata/angular.js that referenced this issue Mar 1, 2015

refactor($cookieStore): use $browser directly instead of $cookies
This is important in order to avoid the current mechanism where polling is required to get the latest cookies and digest is required to flush the latest changes.

Closes #6411
Closes #7631

shahata added a commit to shahata/angular.js that referenced this issue Mar 1, 2015

refactor($cookieStore): use $browser directly instead of $cookies
This is important in order to avoid the current mechanism where polling is required to get the latest cookies and digest is required to flush the latest changes.

Closes #6411
Closes #7631

shahata added a commit to shahata/angular.js that referenced this issue Mar 2, 2015

fix($cookies): move logic into $cookies and deprecate $cookieStore
The new API on `$cookies` includes:

 * `get`
 * `put`
 * `getObject`
 * `putObject`
 * `getAll`
 * `remove`

The new API no longer polls the browser for changes to the cookies and no longer copy
cookie values onto the `$cookies` object.

The polling is expensive and caused issues with the `$cookies` properties not
synchronizing correctly with the actual browser cookie values.

The reason the polling was originally added was to allow communication between
different tabs, but there are better ways to do this today (for example `localStorage`).

DEPRECATION NOTICE:

`$cookieStore` is now deprecated as all the useful logic
has been moved to `$cookies`, to which `$cookieStore` now simply
delegates calls.

BREAKING CHANGE:

`$cookies` no longer exposes properties that represent the current browser cookie
values. Now you must explicitly call `get` or `getObject` to access the cookie
values. This also means that you can no longer watch the `$cookies` properties for
changes to the browser's cookies.

This feature is generally only needed if a 3rd party library was programmatically
changing the cookies at runtime. If you rely on this then you must either write code that
can react to the 3rd party library making the changes to cookies or implement your own polling
mechanism.

Closes #6411
Closes #7631

shahata added a commit to shahata/angular.js that referenced this issue Mar 2, 2015

feat($cookies): move logic into $cookies and deprecate $cookieStore
The new API on `$cookies` includes:

 * `get`
 * `put`
 * `getObject`
 * `putObject`
 * `getAll`
 * `remove`

The new API no longer polls the browser for changes to the cookies and no longer copy
cookie values onto the `$cookies` object.

The polling is expensive and caused issues with the `$cookies` properties not
synchronizing correctly with the actual browser cookie values.

The reason the polling was originally added was to allow communication between
different tabs, but there are better ways to do this today (for example `localStorage`).

DEPRECATION NOTICE:

`$cookieStore` is now deprecated as all the useful logic
has been moved to `$cookies`, to which `$cookieStore` now simply
delegates calls.

BREAKING CHANGE:

`$cookies` no longer exposes properties that represent the current browser cookie
values. Now you must explicitly call `get` or `getObject` to access the cookie
values. This also means that you can no longer watch the `$cookies` properties for
changes to the browser's cookies.

This feature is generally only needed if a 3rd party library was programmatically
changing the cookies at runtime. If you rely on this then you must either write code that
can react to the 3rd party library making the changes to cookies or implement your own polling
mechanism.

Closes #6411
Closes #7631

shahata added a commit to shahata/angular.js that referenced this issue Mar 2, 2015

feat($cookies): move logic into $cookies and deprecate $cookieStore
The new API on `$cookies` includes:

 * `get`
 * `put`
 * `getObject`
 * `putObject`
 * `getAll`
 * `remove`

The new API no longer polls the browser for changes to the cookies and no longer copy
cookie values onto the `$cookies` object.

The polling is expensive and caused issues with the `$cookies` properties not
synchronizing correctly with the actual browser cookie values.

The reason the polling was originally added was to allow communication between
different tabs, but there are better ways to do this today (for example `localStorage`).

DEPRECATION NOTICE:

`$cookieStore` is now deprecated as all the useful logic
has been moved to `$cookies`, to which `$cookieStore` now simply
delegates calls.

BREAKING CHANGE:

`$cookies` no longer exposes properties that represent the current browser cookie
values. Now you must explicitly call `get` or `getObject` to access the cookie
values. This also means that you can no longer watch the `$cookies` properties for
changes to the browser's cookies.

This feature is generally only needed if a 3rd party library was programmatically
changing the cookies at runtime. If you rely on this then you must either write code that
can react to the 3rd party library making the changes to cookies or implement your own polling
mechanism.

Closes #6411
Closes #7631

shahata added a commit to shahata/angular.js that referenced this issue Mar 2, 2015

feat($cookies): move logic into $cookies and deprecate $cookieStore
The new API on `$cookies` includes:

 * `get`
 * `put`
 * `getObject`
 * `putObject`
 * `getAll`
 * `remove`

The new API no longer polls the browser for changes to the cookies and no longer copy
cookie values onto the `$cookies` object.

The polling is expensive and caused issues with the `$cookies` properties not
synchronizing correctly with the actual browser cookie values.

The reason the polling was originally added was to allow communication between
different tabs, but there are better ways to do this today (for example `localStorage`).

DEPRECATION NOTICE:

`$cookieStore` is now deprecated as all the useful logic
has been moved to `$cookies`, to which `$cookieStore` now simply
delegates calls.

BREAKING CHANGE:

`$cookies` no longer exposes properties that represent the current browser cookie
values. Now you must explicitly the methods described above to access the cookie
values. This also means that you can no longer watch the `$cookies` properties for
changes to the browser's cookies.

This feature is generally only needed if a 3rd party library was programmatically
changing the cookies at runtime. If you rely on this then you must either write code that
can react to the 3rd party library making the changes to cookies or implement your own polling
mechanism.

Closes #6411
Closes #7631

shahata added a commit to shahata/angular.js that referenced this issue Mar 2, 2015

feat($cookies): move logic into $cookies and deprecate $cookieStore
The new API on `$cookies` includes:

 * `get`
 * `put`
 * `getObject`
 * `putObject`
 * `getAll`
 * `remove`

The new API no longer polls the browser for changes to the cookies and no longer copy
cookie values onto the `$cookies` object.

The polling is expensive and caused issues with the `$cookies` properties not
synchronizing correctly with the actual browser cookie values.

The reason the polling was originally added was to allow communication between
different tabs, but there are better ways to do this today (for example `localStorage`).

DEPRECATION NOTICE:

`$cookieStore` is now deprecated as all the useful logic
has been moved to `$cookies`, to which `$cookieStore` now simply
delegates calls.

BREAKING CHANGE:

`$cookies` no longer exposes properties that represent the current browser cookie
values. Now you must explicitly the methods described above to access the cookie
values. This also means that you can no longer watch the `$cookies` properties for
changes to the browser's cookies.

This feature is generally only needed if a 3rd party library was programmatically
changing the cookies at runtime. If you rely on this then you must either write code that
can react to the 3rd party library making the changes to cookies or implement your own polling
mechanism.

Closes #6411
Closes #7631

shahata added a commit to shahata/angular.js that referenced this issue Mar 2, 2015

feat($cookies): move logic into $cookies and deprecate $cookieStore
The new API on `$cookies` includes:

 * `get`
 * `put`
 * `getObject`
 * `putObject`
 * `getAll`
 * `remove`

The new API no longer polls the browser for changes to the cookies and no longer copy
cookie values onto the `$cookies` object.

The polling is expensive and caused issues with the `$cookies` properties not
synchronizing correctly with the actual browser cookie values.

The reason the polling was originally added was to allow communication between
different tabs, but there are better ways to do this today (for example `localStorage`).

DEPRECATION NOTICE:

`$cookieStore` is now deprecated as all the useful logic
has been moved to `$cookies`, to which `$cookieStore` now simply
delegates calls.

BREAKING CHANGE:

`$cookies` no longer exposes properties that represent the current browser cookie
values. Now you must explicitly the methods described above to access the cookie
values. This also means that you can no longer watch the `$cookies` properties for
changes to the browser's cookies.

This feature is generally only needed if a 3rd party library was programmatically
changing the cookies at runtime. If you rely on this then you must either write code that
can react to the 3rd party library making the changes to cookies or implement your own polling
mechanism.

Closes #6411
Closes #7631

@shahata shahata closed this in 38fbe3e Mar 2, 2015

hansmaad pushed a commit to hansmaad/angular.js that referenced this issue Mar 10, 2015

feat($cookies): move logic into $cookies and deprecate $cookieStore
The new API on `$cookies` includes:

 * `get`
 * `put`
 * `getObject`
 * `putObject`
 * `getAll`
 * `remove`

The new API no longer polls the browser for changes to the cookies and no longer copy
cookie values onto the `$cookies` object.

The polling is expensive and caused issues with the `$cookies` properties not
synchronizing correctly with the actual browser cookie values.

The reason the polling was originally added was to allow communication between
different tabs, but there are better ways to do this today (for example `localStorage`).

DEPRECATION NOTICE:

`$cookieStore` is now deprecated as all the useful logic
has been moved to `$cookies`, to which `$cookieStore` now simply
delegates calls.

BREAKING CHANGE:

`$cookies` no longer exposes properties that represent the current browser cookie
values. Now you must explicitly the methods described above to access the cookie
values. This also means that you can no longer watch the `$cookies` properties for
changes to the browser's cookies.

This feature is generally only needed if a 3rd party library was programmatically
changing the cookies at runtime. If you rely on this then you must either write code that
can react to the 3rd party library making the changes to cookies or implement your own polling
mechanism.

Closes #6411
Closes #7631

netman92 added a commit to netman92/angular.js that referenced this issue Aug 8, 2015

feat($cookies): move logic into $cookies and deprecate $cookieStore
The new API on `$cookies` includes:

 * `get`
 * `put`
 * `getObject`
 * `putObject`
 * `getAll`
 * `remove`

The new API no longer polls the browser for changes to the cookies and no longer copy
cookie values onto the `$cookies` object.

The polling is expensive and caused issues with the `$cookies` properties not
synchronizing correctly with the actual browser cookie values.

The reason the polling was originally added was to allow communication between
different tabs, but there are better ways to do this today (for example `localStorage`).

DEPRECATION NOTICE:

`$cookieStore` is now deprecated as all the useful logic
has been moved to `$cookies`, to which `$cookieStore` now simply
delegates calls.

BREAKING CHANGE:

`$cookies` no longer exposes properties that represent the current browser cookie
values. Now you must explicitly the methods described above to access the cookie
values. This also means that you can no longer watch the `$cookies` properties for
changes to the browser's cookies.

This feature is generally only needed if a 3rd party library was programmatically
changing the cookies at runtime. If you rely on this then you must either write code that
can react to the 3rd party library making the changes to cookies or implement your own polling
mechanism.

Closes #6411
Closes #7631
@darkbasic

This comment has been minimized.

Show comment
Hide comment
@darkbasic

darkbasic Nov 3, 2015

The bug has been closed but $cookies.get() on AngularJS 1.4.7 still doesn't return a promise: how am I supposed to do?

darkbasic commented Nov 3, 2015

The bug has been closed but $cookies.get() on AngularJS 1.4.7 still doesn't return a promise: how am I supposed to do?

@ZachMoreno

This comment has been minimized.

Show comment
Hide comment
@ZachMoreno

ZachMoreno Nov 15, 2015

@darkbasic +1

$cookies.get('authentication').then(function(authCookie) {
    var userID = authCookie.id;
});

returns

TypeError: $cookies.get(...).then is not a function

in AngularJS v1.4.7

ZachMoreno commented Nov 15, 2015

@darkbasic +1

$cookies.get('authentication').then(function(authCookie) {
    var userID = authCookie.id;
});

returns

TypeError: $cookies.get(...).then is not a function

in AngularJS v1.4.7

@lgalfaso

This comment has been minimized.

Show comment
Hide comment
@lgalfaso

lgalfaso Nov 15, 2015

Member

@ZachMoreno $cookies.get('authentication') is a synchronous function

Member

lgalfaso commented Nov 15, 2015

@ZachMoreno $cookies.get('authentication') is a synchronous function

@firatkucuk

This comment has been minimized.

Show comment
Hide comment
@firatkucuk

firatkucuk Mar 6, 2017

I confirm this issue. It must be an async function. It refreshes too late.

firatkucuk commented Mar 6, 2017

I confirm this issue. It must be an async function. It refreshes too late.

@asim-qb

This comment has been minimized.

Show comment
Hide comment
@asim-qb

asim-qb Apr 20, 2017

Any update on this? Having a timeout is the only workaround?

asim-qb commented Apr 20, 2017

Any update on this? Having a timeout is the only workaround?

@gkalpak

This comment has been minimized.

Show comment
Hide comment
@gkalpak

gkalpak Apr 20, 2017

Member

As already mentioned, the $cookies methods are synchronous, so you shouldn't need a timeout. If you are running into problems, please submit a new issue providing a reproduction of the problem, so we can investigate).

Member

gkalpak commented Apr 20, 2017

As already mentioned, the $cookies methods are synchronous, so you shouldn't need a timeout. If you are running into problems, please submit a new issue providing a reproduction of the problem, so we can investigate).

@WolfElkan

This comment has been minimized.

Show comment
Hide comment
@WolfElkan

WolfElkan May 2, 2017

Here is a reproduction of the problem. Loading this server creates a login/registration application but the page must be refreshed after clicking Login or Logout before the new Partial will be loaded.

WolfElkan commented May 2, 2017

Here is a reproduction of the problem. Loading this server creates a login/registration application but the page must be refreshed after clicking Login or Logout before the new Partial will be loaded.

@gkalpak

This comment has been minimized.

Show comment
Hide comment
@gkalpak

gkalpak May 2, 2017

Member

@WolfElkan, this does not demonstrate that $cookies.set() is asynchronous. The code you linked to doesn't even use $cookies.set() afaict.

Member

gkalpak commented May 2, 2017

@WolfElkan, this does not demonstrate that $cookies.set() is asynchronous. The code you linked to doesn't even use $cookies.set() afaict.

@OrganicCat

This comment has been minimized.

Show comment
Hide comment
@OrganicCat

OrganicCat Jul 3, 2017

I run into this issue if I have an interceptor. So the basic code goes like so:

$cookies.putObject('wowcookie', mycookie);
            
$state.go('home');

The interceptor expects the cookie to exist, however, it does not always. This drove us a little nuts trying to figure out why the cookie was appearing, however unavailable. Throwing $state into a timeout of about 250ms resolves this. Although the timeout could vary I'm sure.

OrganicCat commented Jul 3, 2017

I run into this issue if I have an interceptor. So the basic code goes like so:

$cookies.putObject('wowcookie', mycookie);
            
$state.go('home');

The interceptor expects the cookie to exist, however, it does not always. This drove us a little nuts trying to figure out why the cookie was appearing, however unavailable. Throwing $state into a timeout of about 250ms resolves this. Although the timeout could vary I'm sure.

@gkalpak

This comment has been minimized.

Show comment
Hide comment
@gkalpak

gkalpak Jul 4, 2017

Member

As already mentioned, all calls and expression that happen as part of putObject are synchronous. The only thing that I can think of is that the browser somehow delays the document.cookie = 'some value' operation, but I couldn't reproduce a situation where document.cookie is not updated synchronously.

Again, without a reproduction of the issue, there is nothing we can do. Happy to investigate if someone can provide a repro.

Member

gkalpak commented Jul 4, 2017

As already mentioned, all calls and expression that happen as part of putObject are synchronous. The only thing that I can think of is that the browser somehow delays the document.cookie = 'some value' operation, but I couldn't reproduce a situation where document.cookie is not updated synchronously.

Again, without a reproduction of the issue, there is nothing we can do. Happy to investigate if someone can provide a repro.

@OrganicCat

This comment has been minimized.

Show comment
Hide comment
@OrganicCat

OrganicCat Jul 5, 2017

I was able to reproduce, but my problem was a bug on my end. The interceptor was a factory and we accidentally set the $cookie result outside the response function. Since that variable was not accessible, it was really a shot in the dark regarding when the $cookie would be set. By setting a timeout, it would guarantee the $cookie was set BEFORE the request function was called.

Basically, the $cookie was not updating due to it sitting in a factory as a private variable which didn't propagate to the function expecting it, but it WAS eventually set so it looked like it was set, but in reality it wasn't updated before the request.

Resolution: Set the cookie in the same function as it's being used (or get).

OrganicCat commented Jul 5, 2017

I was able to reproduce, but my problem was a bug on my end. The interceptor was a factory and we accidentally set the $cookie result outside the response function. Since that variable was not accessible, it was really a shot in the dark regarding when the $cookie would be set. By setting a timeout, it would guarantee the $cookie was set BEFORE the request function was called.

Basically, the $cookie was not updating due to it sitting in a factory as a private variable which didn't propagate to the function expecting it, but it WAS eventually set so it looked like it was set, but in reality it wasn't updated before the request.

Resolution: Set the cookie in the same function as it's being used (or get).

@pbrain19

This comment has been minimized.

Show comment
Hide comment
@pbrain19

pbrain19 Nov 8, 2017

still happening. Anyone got an update on that callback?

pbrain19 commented Nov 8, 2017

still happening. Anyone got an update on that callback?

@gkalpak

This comment has been minimized.

Show comment
Hide comment
@gkalpak
Member

gkalpak commented Nov 10, 2017

@darkbasic

This comment has been minimized.

Show comment
Hide comment
@darkbasic

darkbasic Nov 10, 2017

This was indeed an issue because I clearly remember I had to workaround it a couple of years ago. Since I don't use AngularJS anymore I'm no more interested to see it fixed.
If someone else still cares I suggest you to follow @gkalpak suggestion: upload an example project where the issue can be reproduced and you'll see it fixed.

darkbasic commented Nov 10, 2017

This was indeed an issue because I clearly remember I had to workaround it a couple of years ago. Since I don't use AngularJS anymore I'm no more interested to see it fixed.
If someone else still cares I suggest you to follow @gkalpak suggestion: upload an example project where the issue can be reproduced and you'll see it fixed.

@pbrain19

This comment has been minimized.

Show comment
Hide comment
@pbrain19

pbrain19 Nov 10, 2017

Well it appears that it is not isolated to this angular lib. https://github.com/gsklee/ngStorage also experiences and recommends a solution to this issue. (read at the bottom of the readme)

pbrain19 commented Nov 10, 2017

Well it appears that it is not isolated to this angular lib. https://github.com/gsklee/ngStorage also experiences and recommends a solution to this issue. (read at the bottom of the readme)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment