Skip to content

Conversation

@chr-hertel
Copy link
Member

Goal was to shift that json_decode away from construct, but maybe a bit opinionated, and arguable when it comes to robustness due to the referenced array handling atm, but not a big fan of doing work like that in the constructor.

@chr-hertel chr-hertel added the Server Issues & PRs related to the Server component label Feb 8, 2026
@chr-hertel chr-hertel changed the title Shift away session decode from constructor [Server] Shift away session decode from constructor Feb 8, 2026
@chr-hertel chr-hertel force-pushed the refactor-session-construct branch from 6fb3204 to 0751310 Compare February 8, 2026 12:17
@chr-hertel chr-hertel added this to the 0.4.0 milestone Feb 8, 2026
@Nyholm Nyholm added the breaking change Breaking the Backwards Compatibility Promise label Feb 8, 2026
Nyholm
Nyholm previously approved these changes Feb 8, 2026
Copy link
Contributor

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this PR.

I added a small small test

public function __construct(
protected SessionStoreInterface $store,
protected Uuid $id = new UuidV4(),
protected array $data = [],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parameter was used only if we could not read from the store...

Seams weird. It is a BC break to remove it (I've updated the label)

@Nyholm Nyholm merged commit c19d550 into main Feb 8, 2026
18 checks passed
@Nyholm
Copy link
Contributor

Nyholm commented Feb 8, 2026

Thank you

@Nyholm Nyholm deleted the refactor-session-construct branch February 8, 2026 14:49
@chr-hertel
Copy link
Member Author

Thanks for taking care @Nyholm 🙏

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

Labels

breaking change Breaking the Backwards Compatibility Promise Server Issues & PRs related to the Server component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants