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

Jump to bottom

setCookie() expire type issue #233

Closed
lukaspijak opened this issue Feb 5, 2024 · 3 comments · Fixed by #237
Closed

setCookie() expire type issue #233

lukaspijak opened this issue Feb 5, 2024 · 3 comments · Fixed by #237

Comments

@lukaspijak
Copy link
Contributor

Version: 3.3.0

Bug Description

Hello @dg,

in interface Nette\Http\IResponse you add to setCookie method to expiration parameter union type ?int. But in documentation you can use string date representation. For example in Nette\Bridges\SecurityHttp\CookieStorage has property $cookieExpiration union type ?string, but on line 51 you use ?string in setCookie, it can cause type error.

Will it be possible to specify expiration using string|int|null in the future, or will everything be refactored to ?int?
@mabar
Copy link
Contributor

mabar commented Feb 5, 2024

Ideally it should be string|int|\DateTimeInterface|null, as in the Response implementation.
Or at least add it as a BC break in release notes, please

spaze added a commit to spaze/michalspacek.cz that referenced this issue Feb 6, 2024
@spaze
Update packages
366eb10 
Skip nette/http because of nette/http#233

 - contributte/translation updated from v2.0.3 to v2.0.4 patch
   See changes: contributte/translation@v2.0.3...v2.0.4
   Release notes: https://github.com/contributte/translation/releases/tag/v2.0.4

 - nette/di updated from v3.1.8 to v3.2.0 minor
   See changes: nette/di@v3.1.8...v3.2.0
   Release notes: https://github.com/nette/di/releases/tag/v3.2.0

 - phpstan/phpstan updated from 1.10.56 to 1.10.57 patch
   See changes: phpstan/phpstan@1.10.56...1.10.57
   Release notes: https://github.com/phpstan/phpstan/releases/tag/1.10.57

 - roave/security-advisories updated from dev-latest@f81bd7c to dev-latest@f987aa7
   See changes: Roave/SecurityAdvisories@f81bd7c...f987aa7

 - symfony/cache updated from v7.0.2 to v7.0.3 patch
   See changes: symfony/cache@v7.0.2...v7.0.3
   Release notes: https://github.com/symfony/cache/releases/tag/v7.0.3

 - symfony/config updated from v6.4.0 to v7.0.3 major
   See changes: symfony/config@v6.4.0...v7.0.3
   Release notes: https://github.com/symfony/config/releases/tag/v7.0.3

 - symfony/filesystem updated from v7.0.0 to v7.0.3 patch
   See changes: symfony/filesystem@v7.0.0...v7.0.3
   Release notes: https://github.com/symfony/filesystem/releases/tag/v7.0.3

 - symfony/translation updated from v6.4.2 to v7.0.3 major
   See changes: symfony/translation@v6.4.2...v7.0.3
   Release notes: https://github.com/symfony/translation/releases/tag/v7.0.3

 - symfony/var-exporter updated from v7.0.2 to v7.0.3 patch
   See changes: symfony/var-exporter@v7.0.2...v7.0.3
   Release notes: https://github.com/symfony/var-exporter/releases/tag/v7.0.3
@spaze spaze mentioned this issue Feb 6, 2024
Merged
spaze added a commit to spaze/michalspacek.cz that referenced this issue Feb 6, 2024
@spaze
Update packages (#274)
81b7cc2 
Skip nette/http because of nette/http#233

 - contributte/translation updated from v2.0.3 to v2.0.4 patch
   See changes: contributte/translation@v2.0.3...v2.0.4
   Release notes: https://github.com/contributte/translation/releases/tag/v2.0.4

 - nette/di updated from v3.1.8 to v3.2.0 minor
   See changes: nette/di@v3.1.8...v3.2.0
   Release notes: https://github.com/nette/di/releases/tag/v3.2.0

 - phpstan/phpstan updated from 1.10.56 to 1.10.57 patch
   See changes: phpstan/phpstan@1.10.56...1.10.57
   Release notes: https://github.com/phpstan/phpstan/releases/tag/1.10.57

 - roave/security-advisories updated from dev-latest@f81bd7c to dev-latest@f987aa7
   See changes: Roave/SecurityAdvisories@f81bd7c...f987aa7

 - symfony/cache updated from v7.0.2 to v7.0.3 patch
   See changes: symfony/cache@v7.0.2...v7.0.3
   Release notes: https://github.com/symfony/cache/releases/tag/v7.0.3

 - symfony/config updated from v6.4.0 to v7.0.3 major
   See changes: symfony/config@v6.4.0...v7.0.3
   Release notes: https://github.com/symfony/config/releases/tag/v7.0.3

 - symfony/filesystem updated from v7.0.0 to v7.0.3 patch
   See changes: symfony/filesystem@v7.0.0...v7.0.3
   Release notes: https://github.com/symfony/filesystem/releases/tag/v7.0.3

 - symfony/translation updated from v6.4.2 to v7.0.3 major
   See changes: symfony/translation@v6.4.2...v7.0.3
   Release notes: https://github.com/symfony/translation/releases/tag/v7.0.3

 - symfony/var-exporter updated from v7.0.2 to v7.0.3 patch
   See changes: symfony/var-exporter@v7.0.2...v7.0.3
   Release notes: https://github.com/symfony/var-exporter/releases/tag/v7.0.3
spaze added a commit to spaze/michalspacek.cz that referenced this issue Feb 16, 2024
@spaze
Update nette/http to v3.2.4, not to 3.3.0
dbcbd13 
Because of nette/http#233
spaze added a commit to spaze/nette-http that referenced this issue Mar 22, 2024
@spaze
Add string and DateTimeInterface to setCookie expire param
b133911 
In version 3.2.x and older, this was the type in the docblock.

Close nette#233
@spaze spaze mentioned this issue Mar 22, 2024
Merged
@spaze
Copy link
Contributor

spaze commented Mar 22, 2024

I have added the type back in #237, let's see if it will be merged and released :-)

@spaze
Copy link
Contributor

spaze commented Nov 4, 2024 •
edited
Loading

Fyi, the fix in #237 has now been merged, pending release. And now also released in 3.3.1.

@dg dg closed this as completed Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add DateTimeInterface to setCookie $expire param spaze/nette-http
4 participants
@dg @spaze @lukaspijak @mabar