feat: use logger instead of console where possible - #17818
Open
florian-lefebvre wants to merge 13 commits into
Open
feat: use logger instead of console where possible#17818florian-lefebvre wants to merge 13 commits into
florian-lefebvre wants to merge 13 commits into
Conversation
🦋 Changeset detectedLatest commit: 5e48ac5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 425 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
the
pkg: astro
github-actions
Bot
added
the
pkg: integration
`virtual:astro:manifest` statically imports `virtual:astro:pages`, and the runtime logger made `astro:content` reach the ambient manifest transitively (astro:content -> assets/runtime -> runtime/server -> astro-global -> getGlobalLogger -> manifest/ambient). Loading the content config during sync runs in the `astro` environment, where the pages plugin did not apply, so the manifest failed to resolve its page map and content type generation crashed. The serialized-manifest and routes plugins already cover `astro`; this brings the pages plugin in line with them. Page modules are imported lazily from the generated map, so evaluating it in the runnable dev environment is cheap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CUdtfZXucgDyU666b6aCRG
Unit tests for `getGlobalLogger()` (manifest logger vs. console fallback, level handling, destination swaps, identity) and `astroToRuntimeLogger()`. Integration coverage through a new `runtime-logger` fixture whose middleware entrypoint calls `getEntry()` at module scope, outside any request: with no request state to carry a logger, the warning only reaches the user-configured destination if `getGlobalLogger()` resolves the ambient manifest's logger. The fixture also exercises the render-time `set:html`-through-spread and hydration-directive-on-an-Astro-component warnings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CUdtfZXucgDyU666b6aCRG
Merging this PR will degrade performance by 12.96%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | Build: full server site |
1.4 s | 1.6 s | -12.96% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing feat/runtime-logger (5e48ac5) with main (157c500)
`getGlobalLogger()` reaches the ambient manifest, and `virtual:astro:manifest` statically imports `virtual:astro:renderers` and `virtual:astro:pages`. Calling it from `astro/runtime/server` — which every compiled page imports — therefore connected every page's module graph to every renderer and every other page, and Astro's CSS crawling started injecting unrelated styles. `@astrojs/vue`'s app-entrypoint CSS test caught it: a page with no Vue component was served the appEntrypoint's global stylesheet. Revert `astro-global`, `render/util` and `render/astro/instance` to `console`, and drop the integration coverage for those three warnings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CUdtfZXucgDyU666b6aCRG
Replaces `getGlobalLogger()`, which reached the logger through the ambient manifest. `virtual:astro:manifest` statically imports `virtual:astro:renderers` and `virtual:astro:pages`, so any module a page imports that reaches the manifest drags every renderer and every page into that page's module graph — and Astro's CSS crawling then injects unrelated styles. `@astrojs/vue`'s app-entrypoint CSS test caught the first version of this through `astro/runtime/server`; `astro:content` had the same defect for any project combining content collections with a renderer that has an `appEntrypoint`. `virtual:astro:logger` is a leaf — it imports only the user's configured destination — so `astro:content` builds its logger from that instead, and the content runtime factories take it as a parameter. The destination is the same instance the manifest's `logger()` thunk resolves, so content logs still reach the user's destination; only the AstroLogger wrapper differs. The logger virtual module is now always registered (exporting `null` plus the resolved level when unconfigured) so the static `astro:content` template can import it unconditionally, and `getGlobalLogger()` is gone. Also reverts the pages-plugin `astro` environment fix: it only existed because `astro:content` pulled the manifest into content sync, which it no longer does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CUdtfZXucgDyU666b6aCRG
Minor bumps for the two public signature changes — the `logger` parameter on `LocalImageService.transform()` and on the cache provider `onRequest()` context — plus patches for the console-to-logger routing in astro and the Vercel dev image service that has to forward the new argument. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CUdtfZXucgDyU666b6aCRG
The loader schema warning moved from `console.warn` to `logger.warn`, so the test's console patch no longer saw it. Capture the messages with an `AstroLogger` destination passed via `_logger` instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CUdtfZXucgDyU666b6aCRG
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
`stripInternal` keeps `_logger` out of the emitted declarations, so the sync tests need to suppress the type error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CUdtfZXucgDyU666b6aCRG
florian-lefebvre
changed the title
florian-lefebvre
marked this pull request as ready for review
August 26, 2026 08:23
Princesseuh
reviewed
Aug 27, 2026
matthewp
approved these changes
Aug 27, 2026
Extends the `logger` argument beyond `transform()`: `getURL`, `getSrcSet`, `getHTMLAttributes`, `validateOptions`, `getRemoteSize`, and `parseURL` now receive Astro's runtime logger too, so a custom service can log from any hook through the configured destination instead of the console. The hooks reached from `getImage()` had no logger to forward, so the generated `astro:assets` and `virtual:astro:get-image` modules build one from `virtual:astro:logger` and pass it in — the same approach `astro:content` uses. That module is a leaf, so reaching the user's destination through it cannot drag every renderer and page into an importer's module graph the way the manifest would. The client branch gets a console shim instead: there is no configured destination in the browser, and pulling the logger implementation in just for `inferRemoteSize()` would be dead weight. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
|
@Princesseuh done in 5e48ac5 |
Princesseuh
approved these changes
Aug 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
consolecalls with the logger insteadTesting
Added
Docs