Load logging in init templates#727
Conversation
Add startup wiring for generated Nitro and Next.js applications so their LogTape configuration modules are actually loaded before Fedify handles requests. Nitro projects now get a server plugin, while Next.js projects get an instrumentation register hook scoped to the Node.js runtime. Add regression coverage for both framework initializers and document the fix in the changelog. Fixes fedify-dev#725 Assisted-by: Codex:gpt-5.5
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Pull request overview
This PR fixes @fedify/init scaffolding for Nitro and Next.js so the generated logging.ts configuration is actually loaded during server startup, ensuring Fedify uses the intended LogTape configuration (per #725).
Changes:
- Nitro: add a server plugin that imports the generated
server/logging.tsat startup. - Next.js: add an
instrumentation.tsregister()hook that loadslogging.tsin the Node.js runtime. - Add tests asserting the generated templates include the expected wiring, and document the fix in the changelog.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/init/src/webframeworks.ts | Wires new Nitro plugin + Next instrumentation into generated file sets. |
| packages/init/src/webframeworks.test.ts | Adds template-level tests to assert logging wiring exists. |
| packages/init/src/templates/nitro/server/plugins/logging.ts.tpl | New Nitro plugin template that imports server/logging.ts. |
| packages/init/src/templates/next/instrumentation.ts.tpl | New Next.js instrumentation hook to import logging.ts in Node runtime. |
| CHANGES.md | Documents the init template fix for 2.0.15 and links #725. |
There was a problem hiding this comment.
Code Review
This pull request updates the @fedify/init package to ensure that generated logging.ts files are correctly loaded during server startup for Nitro and Next.js templates. For Nitro, a new server plugin is introduced, while for Next.js, an instrumentation.ts file with a register() hook is added. The PR also includes corresponding updates to the framework configuration and new regression tests to verify the presence and content of these files. I have no feedback to provide.
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 4 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Avoid reading process.env directly in the generated Next.js instrumentation hook. Using globalThis.process?.env keeps the hook safe when instrumentation is evaluated outside the Node.js runtime, while still loading the LogTape setup for Node.js requests. Updates the initializer regression test to assert the guarded runtime check. Review comment: fedify-dev#727 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request fixes the Nitro and Next.js project templates to ensure their generated logging.ts files are loaded during server startup. Nitro projects now include a server plugin that imports the LogTape configuration, and Next.js projects include an instrumentation.ts file with a register() hook for the same purpose. The PR also adds regression tests and updates the changelog. I have no feedback to provide.
There was a problem hiding this comment.
Pull request overview
This PR fixes fedify init scaffolding for Nitro and Next.js by ensuring the generated logging.ts (LogTape configuration) is actually loaded during server startup, matching user expectations and resolving #725.
Changes:
- Nitro: add a Nitro server plugin that imports
server/logging.tsat startup. - Next.js: add
instrumentation.tsthat imports./loggingfromregister()when running in the Node.js runtime. - Add unit tests to assert the generated templates include the new startup wiring, and document the fix in the changelog.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/init/src/webframeworks.ts | Wires new Nitro plugin and Next.js instrumentation template into generated file sets. |
| packages/init/src/webframeworks.test.ts | Adds tests asserting Nitro/Next templates load logging at startup. |
| packages/init/src/templates/nitro/server/plugins/logging.ts.tpl | New Nitro plugin that imports server/logging.ts via Nitro’s plugin startup path. |
| packages/init/src/templates/next/instrumentation.ts.tpl | New Next.js register() hook that conditionally imports logging in Node.js runtime. |
| CHANGES.md | Adds changelog entry describing the @fedify/init template fix and links #725. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79443df343
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Use the documented process.env.NEXT_RUNTIME guard in the generated Next.js instrumentation hook. Next.js relies on this pattern for runtime-specific imports, so the Node-only logging module is not pulled into Edge bundles. Updates the initializer regression test to assert the documented runtime check. Review comment: fedify-dev#727 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request modifies the @fedify/init package to ensure that the generated logging.ts files are automatically loaded during server startup for Nitro and Next.js templates. Nitro projects now include a server plugin for LogTape configuration, while Next.js projects utilize an instrumentation.ts file with a register() hook. The changes include new template files, updates to the framework configurations, and regression tests. I have no feedback to provide.
There was a problem hiding this comment.
Pull request overview
This PR fixes @fedify/init scaffolding so framework templates that generate a logging.ts configuration also ensure it is executed during server startup, restoring expected LogTape/Fedify logging behavior in generated apps.
Changes:
- Nitro: adds a
server/plugins/logging.tsplugin to importserver/logging.tsduring Nitro startup. - Next.js: adds
instrumentation.tsto import./loggingvia Next’sregister()hook when running in the Node.js runtime. - Adds tests to assert the generated templates include the expected wiring, and documents the fix in
CHANGES.md.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/init/src/webframeworks.ts | Wires new Nitro plugin and Next.js instrumentation file into generated template outputs. |
| packages/init/src/webframeworks.test.ts | Adds regression tests asserting the new logging-wiring files are generated and contain expected imports. |
| packages/init/src/templates/nitro/server/plugins/logging.ts.tpl | New Nitro server plugin template that imports the generated logging configuration. |
| packages/init/src/templates/next/instrumentation.ts.tpl | New Next.js instrumentation template intended to import logging config during Node runtime startup. |
| CHANGES.md | Changelog entry documenting the Nitro/Next template fix for issue #725. |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
Fixes #725.
The Nitro and Next.js
fedify inittemplates already generated LogTape configuration files, but those files were not imported by the generated server entrypoints. As a result, Fedify logs used the default LogTape state instead of the scaffolded configuration.This PR adds packages/init/src/templates/nitro/server/plugins/logging.ts.tpl, which imports server/logging.ts through Nitro’s server plugin startup path.
This PR also adds packages/init/src/templates/next/instrumentation.ts.tpl, which imports logging.ts from Next.js'
register()hook whenNEXT_RUNTIMEisnodejs.Astro and SolidStart are intentionally left out here because they were introduced after the 2.0.x line. This keeps the fix suitable for the 2.0 maintenance branch; the 2.1.x branch can carry the same fix forward and handle those integrations separately.
Tests
deno task checkin packages/initpnpm --filter @fedify/init testdeno check packages/init/src/webframeworks.test.tsdeno run -A packages/init/src/test/execute.ts init /tmp/fedify-init-725-nitro -w nitro -p npm -k in-memory -m in-process --dry-rundeno run -A packages/init/src/test/execute.ts init /tmp/fedify-init-725-next -w next -p npm -k in-memory -m in-process --dry-run