Skip to content

Improve description of locals on guides/middleware#5522

Merged
sarah11918 merged 6 commits into
withastro:mainfrom
Fryuni:middleware-locals
Dec 2, 2023
Merged

Improve description of locals on guides/middleware#5522
sarah11918 merged 6 commits into
withastro:mainfrom
Fryuni:middleware-locals

Conversation

@Fryuni
Copy link
Copy Markdown
Member

@Fryuni Fryuni commented Nov 29, 2023

Description (required)

Change the comments and description of the locals object on guides/middleware.

Related issues & labels (optional)

@vercel
Copy link
Copy Markdown

vercel Bot commented Nov 29, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview Dec 2, 2023 5:41pm

Copy link
Copy Markdown
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Thank you for jumping in and helping us get this example just right, @Fryuni ! I'm not sure we're quite there yet, so I've left my thoughts so we can keep workshopping it. See what you think!

### Storing data in `context.locals`

`context.locals` is an object containing data from a `Response` that can be manipulated inside middleware.
`context.locals` is an object that can be manipulated inside the middleware. Adapters _might_ add data to it.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"Adapters might add data to it" is a little too vague to be helpful.

It's not clear whether the reader should take this as some kind of warning (i.e. if you're writing your own middleware , beware that there might be something else that's also manipulating the data in context.locals), or it's being used as an example of something that manipulates context.locals to provide real-usage context, or something else entirely.... fun fact! Did you know...! 😄

Can you reword that statement in a way that makes it clear why they are being told that?

Copy link
Copy Markdown
Member Author

@Fryuni Fryuni Nov 29, 2023

Choose a reason for hiding this comment

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

I think this could be added on a tip. What do you think of removing that last sentence and adding this:

:::tip[Adapter-provided properties]
Adapters may provide extra runtime information and functionality through the `locals` object.
For example, SSR adapters may include information about the request or runtime.
:::

We can link to some examples. Of the official integrations, 4 modify the locals object.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I still don't know why you're telling me this, though? Again, it's not clear why I should care about this, whether it's something for me to watch out for...

Copy link
Copy Markdown
Member Author

@Fryuni Fryuni Nov 29, 2023

Choose a reason for hiding this comment

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

I think it is a mix of everything. It is something to be aware:

  • An integration may provide something that you are checking manually
  • Something you write might override a value used by an integration
  • Information that would be hard to access or infer otherwise might already be included there (like CloudFlare's runtime)

Maybe it would be clearer with examples

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OK, gotcha! No, I don't want examples here to clutter this up. The reasoning/why I should care is way more important. I think what you mentioned above is the kind of content that would be good to go inside a note!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I sent a new version with a different tip. WDYT?

Comment thread src/content/docs/en/guides/middleware.mdx Outdated
Comment thread src/content/docs/en/guides/middleware.mdx Outdated
Comment thread src/content/docs/en/guides/middleware.mdx Outdated
@sarah11918
Copy link
Copy Markdown
Member

So @Fryuni , happy if you add a new note based on the "why I need to know/care" ideas and then adjust the two code samples accordingly!

Let me know when you'd like me to take another look!

There is a separate section explaining and demonstrating how to use the
result of `next()` to transform the response after rendering.
Copy link
Copy Markdown
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Looks great, @Fryuni ! Thanks for your patience and cooperation during a particularly stressful docs time! 😅

Happy to have this improvement in the docs! 🥳

@sarah11918 sarah11918 added the improve or update documentation Enhance / update existing documentation (e.g. add example, improve description, update for changes) label Dec 2, 2023
@sarah11918 sarah11918 merged commit 5e4f070 into withastro:main Dec 2, 2023
@Fryuni Fryuni deleted the middleware-locals branch December 2, 2023 23:17
yanthomasdev added a commit that referenced this pull request Dec 3, 2023
Update French Translation with PR #5522

Co-authored-by: Yan Thomas <61414485+Yan-Thomas@users.noreply.github.com>
ematipico pushed a commit that referenced this pull request Jan 26, 2024
Update French Translation with PR #5522

Co-authored-by: Yan Thomas <61414485+Yan-Thomas@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improve or update documentation Enhance / update existing documentation (e.g. add example, improve description, update for changes)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve locals documentation on Middleware

2 participants