Skip to content

Conversation

mmart1n
Copy link
Contributor

@mmart1n mmart1n commented Mar 21, 2025

Seems to fix #346

image

@DvDruzhkov
Copy link

Hi, this problem is blocker of using cookie service for ssr.
Fix looks good, please tell me when we can upload fix?

@mmart1n
Copy link
Contributor Author

mmart1n commented Mar 26, 2025

Hi, this problem is blocker of using cookie service for ssr. Fix looks good, please tell me when we can upload fix?

I hope that the mainter can review this PR soon and if everything is ok to make new relase.

@chdanielmueller
Copy link

chdanielmueller commented Apr 9, 2025

Hi @pavankjadda

Sorry to ping you here but it would be great if you could review this.
It looks good to me. This would close #346
The issue was introduced by cd89593

Thanks

@chdanielmueller
Copy link

chdanielmueller commented Apr 9, 2025

I now understand why this changed. It has to do with the CommonEngine and the AngularNodeAppEngine.
Maybe this code can be written to support both methods of getting the cookies

(this.request?.headers.cookie ?? this.request?.headers.get('cookie'))

@mmart1n mmart1n requested a review from chdanielmueller April 9, 2025 13:54
Copy link

sonarqubecloud bot commented Apr 9, 2025

@pavankjadda pavankjadda merged commit 28a97e4 into stevermeister:master Apr 13, 2025
2 checks passed
@pavankjadda
Copy link
Collaborator

@mmart1n
Copy link
Contributor Author

mmart1n commented Apr 14, 2025

@mmart1n @chdanielmueller Why do I see this error? https://github.com/stevermeister/ngx-cookie-service/actions/runs/14431934927/job/40467746451#step:5:16

Yep, my bad — it seems that the cookie property on the headers object is not explicitly declared. One way to avoid the error is by using bracket notation like this.request?.headers['cookie'], or alternatively by extending the type in global.d.ts.

However, I believe this is an oversight on Angular's side, and once it's fixed, this.request?.headers.cookie should work as expected. In the meantime, I've submitted another PR using the bracket notation.

Feel free to adjust it or explore another approach if needed — just wanted to get something in place since this feature is needed ASAP.

Thanks in advance! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

this.request.headers.get is no a function
4 participants