Resolve issues related with fedify init#563
Resolve issues related with fedify init#5632chanhaeng wants to merge 16 commits intofedify-dev:mainfrom
fedify init#563Conversation
Summary of ChangesHello @2chanhaeng, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the project initialization process for Fedify. By separating the core initialization logic into a dedicated Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This is a great pull request that significantly improves the project's structure by separating the init functionality into its own package, @fedify/init, and introducing create-fedify-app for easier project scaffolding. The addition of automatic linting setup with @fedify/lint in new projects is a valuable improvement for code quality. The new check for running database services in test-init is also a nice developer experience enhancement.
The code changes are well-organized and the refactoring is clean. I have one suggestion to improve the reliability of the port checking logic in the new test utility to avoid potential false positives. Other than that, the changes look solid. Well done!
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
dahlia
left a comment
There was a problem hiding this comment.
There are some conflicts; could you rebase your commits on the latest main?
|
Just a reminder, there are multiple functions marked as export, but only used internally in general. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This is a significant pull request that refactors the fedify init functionality into a new, modular @fedify/init package and introduces the @fedify/create package for easier project scaffolding. The changes are well-executed, including necessary updates to documentation, build configurations, and paths. I appreciate the addition of automatic linting setup for new projects and the developer-friendly check for running database servers in the test suite.
I've identified a high-severity issue with an unsafe regex that could corrupt generated JSON files, and a medium-severity issue with a fragile port-checking mechanism in the tests. My review includes suggestions to address these points. Overall, this is a great contribution that enhances the project's structure and usability.
There was a problem hiding this comment.
Pull request overview
This PR consolidates and fixes multiple fedify init / test-init issues by extracting the initializer into a standalone @fedify/init package, adding a lightweight project scaffolding CLI (@fedify/create), and updating generated projects to include @fedify/lint by default.
Changes:
- Split initializer logic from
@fedify/cliinto new@fedify/init, and rewire CLI to consume it. - Add
@fedify/createto supportnpm init @fedify/pnpm create @fedifyflows. - Add linting defaults to generated projects and add DB-running notifications for
test-init.
Reviewed changes
Copilot reviewed 58 out of 88 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Add new workspace packages (create, init) |
| pnpm-lock.yaml | Update lockfile for new packages/deps |
| deno.lock | Add Deno lock entries for packages/init |
| deno.json | Include packages/init and update template excludes |
| .github/workflows/main.yaml | Adjust deno publish dry-run flags |
| .hongdown.toml | Expand exclusion patterns |
| .gitignore | Ignore plans/ directory |
| CHANGES.md | Add changelog entries for @fedify/init / @fedify/create |
| AGENTS.md | Document new package directories |
| CONTRIBUTING.md | Mention packages/create / packages/init in package list |
| docs/install.md | Document @fedify/create alternative flow |
| docs/cli.md | Note @fedify/lint + @fedify/create tips |
| packages/fedify/README.md | Add @fedify/create / @fedify/init to package list |
| packages/cli/package.json | Add dependency on @fedify/init |
| packages/cli/deno.json | Remove old init template excludes / task changes |
| packages/cli/tsdown.config.ts | Remove init test entry + template copy hook |
| packages/cli/src/init/mod.ts | Re-export init API from @fedify/init |
| packages/cli/scripts/pack.ts | Update deno compile --include path for templates |
| packages/init/package.json | New @fedify/init package manifest |
| packages/init/deno.json | New Deno config/tasks for @fedify/init |
| packages/init/README.md | New package documentation |
| packages/init/tsdown.config.ts | Build config + copy templates/json into dist |
| packages/init/src/mod.ts | Public exports for initializer API |
| packages/init/src/const.ts | Add DB_TO_CHECK list |
| packages/init/src/command.ts | Inline debug option + export initOptions |
| packages/init/src/lib.ts | Adjust metadata/template resolution for new package |
| packages/init/src/types.ts | Fix RequiredNotNull import location |
| packages/init/src/utils.ts | Add shared utilities (spawn runner, printing, etc.) |
| packages/init/src/webframeworks.ts | Add lint defaults + eslint config generation |
| packages/init/src/ask/mod.ts | New prompt flow composition |
| packages/init/src/ask/dir.ts | Update utils import path |
| packages/init/src/ask/kv.ts | Update utils import path |
| packages/init/src/ask/mq.ts | Update utils import path |
| packages/init/src/ask/pm.ts | Move label-resolution logic locally |
| packages/init/src/ask/wf.ts | New web framework prompt helper |
| packages/init/src/action/mod.ts | Update utils import + doc comment refresh |
| packages/init/src/action/dir.ts | New helper to create target dir |
| packages/init/src/action/utils.ts | Add env stringify + join helpers |
| packages/init/src/action/configs.ts | Add @fedify/lint config + vscode recommendations |
| packages/init/src/action/deps.ts | Update utils import path |
| packages/init/src/action/env.ts | Update utils import path |
| packages/init/src/action/install.ts | Update utils import path |
| packages/init/src/action/notice.ts | Update utils import path |
| packages/init/src/action/patch.ts | Update utils import path |
| packages/init/src/action/precommand.ts | Update utils import path |
| packages/init/src/action/recommend.ts | Update utils import path |
| packages/init/src/action/set.ts | Update utils import path |
| packages/init/src/action/templates.ts | Update utils import path |
| packages/init/src/action/const.ts | Fix PACKAGES_PATH resolution after extraction |
| packages/init/src/json/biome.json | Disable Biome linter (formatter-only) |
| packages/init/src/json/db-to-check.json | Add DB metadata for test-init checks |
| packages/init/src/json/kv.json | New KV store descriptors |
| packages/init/src/json/mq.json | New message queue descriptors |
| packages/init/src/json/pm.json | New package manager descriptors |
| packages/init/src/json/rt.json | New runtime descriptors |
| packages/init/src/json/vscode-settings.json | New VS Code settings template |
| packages/init/src/json/vscode-settings-for-deno.json | New Deno-specific VS Code settings |
| packages/init/src/templates/defaults/eslint.config.ts.tpl | Add eslint config template for @fedify/lint |
| packages/init/src/templates/defaults/federation.ts.tpl | Add federation template |
| packages/init/src/templates/defaults/logging.ts.tpl | Add logging template |
| packages/init/src/templates/express/app.ts.tpl | Add Express app template |
| packages/init/src/templates/express/index.ts.tpl | Add Express entry template |
| packages/init/src/templates/hono/app.tsx.tpl | Add Hono app template |
| packages/init/src/templates/hono/index/bun.ts.tpl | Add Hono Bun entry template |
| packages/init/src/templates/hono/index/deno.ts.tpl | Add Hono Deno entry template |
| packages/init/src/templates/hono/index/node.ts.tpl | Add Hono Node entry template |
| packages/init/src/templates/elysia/index/bun.ts.tpl | Add Elysia Bun entry template |
| packages/init/src/templates/elysia/index/deno.ts.tpl | Add Elysia Deno entry template |
| packages/init/src/templates/elysia/index/node.ts.tpl | Add Elysia Node entry template |
| packages/init/src/templates/next/middleware.ts.tpl | Add Next.js middleware template |
| packages/init/src/templates/nitro/.env.test.tpl | Add Nitro test env template |
| packages/init/src/templates/nitro/nitro.config.ts.tpl | Add Nitro config template |
| packages/init/src/templates/nitro/server/error.ts.tpl | Add Nitro error handler template |
| packages/init/src/templates/nitro/server/middleware/federation.ts.tpl | Add Nitro federation middleware template |
| packages/init/src/test/mod.ts | Adjust test-init runner entry |
| packages/init/src/test/action.ts | Add DB check step + update utils import |
| packages/init/src/test/create.ts | Update execution entrypoint + cwd handling |
| packages/init/src/test/db.ts | New DB port-check notifications |
| packages/init/src/test/execute.ts | New init command execution entrypoint |
| packages/init/src/test/fill.ts | New helper to fill missing test-init options |
| packages/init/src/test/lookup.ts | Update utils import path |
| packages/init/src/test/run.ts | Update utils import path |
| packages/init/src/test/types.ts | Add DbToCheckType type |
| packages/init/src/test/utils.ts | Use rm() + update utils import path |
| packages/create/package.json | New @fedify/create CLI package manifest |
| packages/create/README.md | Add usage/docs for @fedify/create |
| packages/create/tsdown.config.ts | Build config for @fedify/create |
| packages/create/src/mod.ts | CLI entrypoint wrapping @fedify/init |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
packages/init/src/webframeworks.ts:24
defaultDenoDependencies/defaultDevDependenciesare referenced while building thewebFrameworksobject, but they’re declared later in the module. Becauseconstdeclarations aren’t hoisted, this will throw aReferenceErrorduring module initialization. Move thedefault*Dependenciesdeclarations abovewebFrameworks(or inline them) so the module can load.
packages/init/src/utils.ts
Outdated
| } | ||
|
|
||
| export type GeneratedType<T extends Generator> = T extends | ||
| Generator<unknown, infer R, unknown> ? R : never; |
There was a problem hiding this comment.
This looks like it extracts the yield type, but it actually extracts any due to Generator's default type parameters. Might want to double-check if that's intentional currently all the type safety here is coming from manual as casts, not the utility. Also It won't be the problem in runtime since it doesn't use stricter compiler. Just a bit worrying.
There was a problem hiding this comment.
Fixed at abfdbf0!
It was just a wrong code. How did I not notice this problem until now? 😂
| "author": { | ||
| "name": "Hong Minhee", | ||
| "email": "hong@minhee.org", | ||
| "url": "https://hongminhee.org/" | ||
| }, |
There was a problem hiding this comment.
Since @fedify/create is a new package created by you in this PR, the author field should credit you rather than me. Could you update it to your information?
The same applies to packages/init/package.json as well.
| }, | ||
| "test": { | ||
| "exclude": [ | ||
| "src/init/test/**" |
There was a problem hiding this comment.
I think this exclude pattern should be updated?
| - **Message queues**: Deno KV, Redis, PostgreSQL, AMQP | ||
|
|
||
| See the [`@fedify/init`] package or the [Fedify CLI docs] for details on | ||
| available options (`-r`, `-p`, `-w`, `-k`, `-q`, `--dry-run`). |
| optional(or(noHydRun, noDryRun)), | ||
| ), | ||
| { | ||
| brief: message`Test an initializing command .`, |
There was a problem hiding this comment.
There's an unnecessary space right after the period.
There was a problem hiding this comment.
It would be great if it has its engines field.
| "@optique/core": "^0.9.0", | ||
| "@optique/run": "^0.9.0", |
There was a problem hiding this comment.
Since @optique/core and @optique/run are now used across three packages (@fedify/cli, @fedify/init, and @fedify/create), it would be nice to add them to the pnpm catalog in pnpm-workspace.yaml and use "catalog:" in each package.json—consistent with how es-toolkit, @logtape/logtape, etc. are already managed.
| "@optique/core": "jsr:@optique/core@^0.9.0", | ||
| "@optique/run": "jsr:@optique/run@^0.9.0", |
There was a problem hiding this comment.
Similarly on the Deno side, @optique/core and @optique/run in packages/init/deno.json imports could be moved to the root deno.json imports so they are managed at the workspace level—same as @fxts/core, @logtape/logtape, es-toolkit, etc. are already.
Summary
Resolve issues related with
fedify init. This includes:npx create-fedify-app#351initCLI and fix the command #461@fedify/initfrom@fedify/cli#482fedify test-init#485fedify init: Add@fedify/lintto generated projects by default #513Related Issue
Reference the related issue(s) by number, e.g.:
npx create-fedify-app#351initCLI and fix the command #461@fedify/initfrom@fedify/cli#482fedify test-init#485fedify init: Add@fedify/lintto generated projects by default #513fedify initandtest-init#533Changes
There have been several accumulated issues related to
fedify init. To avoid potential conflicts from separate pull requests addressing more than one issue, we have consolidated and resolved them in a single issue, #533.@fedify/initfrom@fedify/cli.create-fedify-appusing@fedify/init.@fedify/lintand its related configurations and files when creating a project with@fedify/init.@fedify/lintwithtest-init, it now notifies the user if related local database servers likeRedisare not running.Benefits
@fedify/initallows for the creation of additional packages or apps, such ascreate-fedify-app.create-fedify-appwithout needing to install@fedify/cli.fedify initandcreate-fedify-appare automatically linted, improving user convenience.test-initnow helps with error resolution by notifying the user if local database servers are not running.Checklist
Did you write a regression test to reproduce the bug (if it's a bug fix)?Did you write some tests for this change (if it's a new feature)?deno task test-allon your machine?Additional Notes
Include any other information, context, or considerations.