Skip to content

feat!: clean up internal symbols usage#14420

Merged
florian-lefebvre merged 5 commits intonextfrom
feat/locals-symbol-dev-only
Sep 23, 2025
Merged

feat!: clean up internal symbols usage#14420
florian-lefebvre merged 5 commits intonextfrom
feat/locals-symbol-dev-only

Conversation

@florian-lefebvre
Copy link
Copy Markdown
Member

@florian-lefebvre florian-lefebvre commented Sep 22, 2025 •
edited
Loading

Changes

  • The astro.locals symbol is only used in dev by official adapters, so it can be removed from here
  • the astro.clientAddress symbol is meant to be used internally only, now that clientAddress can be passed to app.render()

Testing

Should still pass

Docs

Shouldn't be needed as it's an undocumented API

@florian-lefebvre florian-lefebvre added this to the v6.0.0 milestone Sep 22, 2025
@florian-lefebvre florian-lefebvre self-assigned this Sep 22, 2025
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Sep 22, 2025 •
edited
Loading

⚠️ No Changeset found

Latest commit: 74ed9d4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added pkg: integration Related to any renderer integration (scope) pkg: astro Related to the core `astro` package (scope) labels Sep 22, 2025
@florian-lefebvre
Copy link
Copy Markdown
Member Author

florian-lefebvre commented Sep 22, 2025

@sarah11918 let me know if you think this needs docs but I don't think it does!

florian-lefebvre
florian-lefebvre commented Sep 22, 2025
View reviewed changes
packages/integrations/netlify/src/index.ts
Copy link
Copy Markdown
Member Author

@florian-lefebvre florian-lefebvre Sep 22, 2025

Choose a reason for hiding this comment

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

Updated to match cloudflare

@florian-lefebvre florian-lefebvre changed the title feat!: only check for locals symbol in development feat!: clean up internal symbols usage Sep 22, 2025
@sarah11918
Copy link
Copy Markdown
Member

sarah11918 commented Sep 22, 2025

I think this all seems pretty internal, if it's only official adapters (so, integration authors don't need to know/do anything different?) and it's otherwise internal code (so, no Astro project code needs to change.)

So, I'll still want to review the changeset in the end, but likely no docs seems like the right call!

@florian-lefebvre
Copy link
Copy Markdown
Member Author

florian-lefebvre commented Sep 22, 2025 •
edited
Loading

Since it should only be a refactor, I'm thinking about not including a changeset at all

@florian-lefebvre

This comment was marked as outdated.

Sign in to view
@florian-lefebvre florian-lefebvre marked this pull request as draft September 22, 2025 15:35
@florian-lefebvre florian-lefebvre marked this pull request as ready for review September 23, 2025 08:28
@florian-lefebvre florian-lefebvre merged commit 2570c4b into next Sep 23, 2025
18 of 19 checks passed
@florian-lefebvre florian-lefebvre deleted the feat/locals-symbol-dev-only branch September 23, 2025 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@matthewp matthewp matthewp approved these changes

Assignees

@florian-lefebvre florian-lefebvre

Labels

pkg: astro Related to the core `astro` package (scope) pkg: integration Related to any renderer integration (scope)

Projects

None yet

Milestone

v6.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@florian-lefebvre @sarah11918 @matthewp