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.

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

bug / clarification: session.set is pass by reference, leading to edits after calling .set to be persisted #12881

Closed
1 task done
kaytwo opened this issue Jan 2, 2025 · 0 comments · Fixed by #12883
Closed
1 task done
Labels
feat: content layer Related to the Content Layer feature (scope)

Comments

@kaytwo
Copy link
Contributor

kaytwo commented Jan 2, 2025

Astro Info

Astro                    v5.1.1
Node                     v22.12.0
System                   macOS (arm64)
Package Manager          npm
Output                   static
Adapter                  @astrojs/node

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

This line sets the "data" value directly to be the passed in value, so if it's an object it's pass by reference:

Thus if you do something like

const allData = {secret: "word", notSecret: "stuff"}
Astro.session.set("allData", allData);
delete allData.secret
return allData;

The deferred saving of the session data causes the updates after the call to .set() to be persisted to the session store.

What's the expected result?

When Astro does this check:

stringify(value);

It should do the full serialization (including the URL conversion) and save that to #data.value, instead of doing it later:

serialized = stringify(data, {

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-muwk6ati-e1czmh2l?file=src%2Fpages%2Findex.astro

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Jan 2, 2025
@ascorbic ascorbic added feat: content layer Related to the Content Layer feature (scope) and removed needs triage Issue needs to be triaged labels Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: content layer Related to the Content Layer feature (scope)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants