Conversation
- limit add to supported add-ons - drop registry docs, routes, and build path
🦋 Changeset detectedLatest commit: 9f35ed5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughRemoves the ProofKit registry and its templates/docs/API; restricts Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
apps/docs/tests/smoke.test.ts (1)
3-6: Replace the tautological smoke assertion with a behavior check.
expect(true).toBe(true)doesn’t protect against regressions in docs content/routes. Consider asserting a real invariant tied to this PR (e.g., adding-components docs metadata/routing state).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/tests/smoke.test.ts` around lines 3 - 6, The current tautological assertion inside describe("docs") / it("has a smoke test") should be replaced with a real behavior check: import the docs routing/metadata helper (e.g., a function like getDocsRoutes or docsMetadata) or the app router, invoke it in the test and assert the expected invariant introduced by this PR (for example, that a specific route path or metadata key for the new "adding-components" page exists). Update the test body to call that helper and replace expect(true).toBe(true) with an assertion such as expect(routes).toContain("/adding-components") or expect(metadata).toHaveProperty("adding-components"), so the test validates actual docs state rather than a tautology.apps/docs/next.config.ts (1)
24-24: Consider keeping a compatibility redirect for legacy/registry/*URLs.Dropping this redirect entirely can cause link rot for previously shared docs links; a fallback to
/docs/cliwould preserve UX.🔁 Compatibility redirect option
async redirects() { - return [{ source: "/docs", destination: "/docs/cli", permanent: false }]; + return [ + { source: "/docs", destination: "/docs/cli", permanent: false }, + { source: "/registry/:path*", destination: "/docs/cli", permanent: false }, + ]; },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/next.config.ts` at line 24, Add a compatibility redirect for legacy /registry/* URLs by updating the redirects array returned in the redirects() function (the return currently containing { source: "/docs", destination: "/docs/cli", permanent: false }). Insert another redirect entry like { source: "/registry/:path*", destination: "/docs/cli", permanent: false } before or alongside the existing rule so old /registry links fall back to /docs/cli while remaining non-permanent.apps/docs/content/docs/cli/v2.mdx (1)
30-30: Minor style suggestion: simplify "are able to" to "can".This is a minor readability improvement suggested by static analysis.
📝 Suggested wording
-Unfortunately, there is not an easy path for you to upgrade projects that were created with v1 to be fully compatible with v2. But if you are able to setup tailwindcss and shadcn in your existing project, you can still use package-native tools directly alongside ProofKit. +Unfortunately, there is not an easy path for you to upgrade projects that were created with v1 to be fully compatible with v2. But if you can set up tailwindcss and shadcn in your existing project, you can still use package-native tools directly alongside ProofKit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/content/docs/cli/v2.mdx` at line 30, Replace the phrase "are able to setup tailwindcss and shadcn in your existing project" with the simpler and grammatically correct "can set up tailwindcss and shadcn in your existing project" in the sentence that begins "Unfortunately, there is not an easy path..." so the wording is concise and uses the correct verb form ("set up" vs "setup").apps/docs/content/docs/cli/guides/adding-components/index.mdx (1)
16-21: Consider documenting both supported add-on targets.The CLI addon command supports two targets:
webviewerandauth(perpackages/cli/src/cli/add/addon.tsline 39). Currently onlywebvieweris shown as an example. Consider addingauthor listing both supported targets so users discover the full capability.📝 Suggested documentation update
The supported `add` flow is: +**Supported targets:** `webviewer`, `auth` <CliCommand command="proofkit add addon webviewer" /> Use package-native tools or agents for everything else.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/content/docs/cli/guides/adding-components/index.mdx` around lines 16 - 21, The docs example only shows the webviewer target for the "proofkit add addon" command but the CLI supports both "webviewer" and "auth"; update the example in this guide to either list both supported targets or add a second example (e.g., show "proofkit add addon webviewer" and "proofkit add addon auth") so users discover both options; reference the CLI's addon implementation (addon.ts) and the "proofkit add addon" command when making the change.packages/cli/src/cli/add/index.ts (1)
6-12: Remove the unusednoInstalloption from the function signature.The
noInstallparameter is accepted in the function options but never passed torunAddAddonAction. Since the addon installation flow only copies FM add-on files and doesn't perform npm package installation (unlike the init flow), this parameter has no effect and can be removed from the options type signature.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/cli/add/index.ts` around lines 6 - 12, Remove the unused noInstall option from the runAdd function signature: update the options type from { noInstall?: boolean; target?: string } to only include target, and ensure calls to runAdd (and any callers/types) expect options?: { target?: string } so that runAdd passes only options?.target into runAddAddonAction; reference the runAdd function and runAddAddonAction to locate and update the signature and related types/usages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/docs/content/docs/cli/guides/auth/add-on.mdx`:
- Line 97: Fix the wording typo in the sentence that currently reads "FileMaker
Add-on Auth will only setup email/password authentication for you" by changing
"setup" to the verb form "set up"; update the copy in the doc content (the
sentence containing "FileMaker Add-on Auth will only setup email/password
authentication for you, but any other...") so it reads "FileMaker Add-on Auth
will only set up email/password authentication for you..." and leave the rest
unchanged.
In `@packages/cli/src/cli/add/auth.ts`:
- Around line 95-97: There are two inconsistent shadcn messages in this module;
unify them by extracting a single constant (e.g., SHADCN_UNSUPPORTED_MESSAGE)
and use it wherever the code checks settings.ui === "shadcn" and throws (replace
the throw new Error("...") in the auth flow that currently uses "no longer
supported" and the other location that says "not yet supported" to reference
this constant); update the throw sites to throw new
Error(SHADCN_UNSUPPORTED_MESSAGE) so both auth paths show identical text.
---
Nitpick comments:
In `@apps/docs/content/docs/cli/guides/adding-components/index.mdx`:
- Around line 16-21: The docs example only shows the webviewer target for the
"proofkit add addon" command but the CLI supports both "webviewer" and "auth";
update the example in this guide to either list both supported targets or add a
second example (e.g., show "proofkit add addon webviewer" and "proofkit add
addon auth") so users discover both options; reference the CLI's addon
implementation (addon.ts) and the "proofkit add addon" command when making the
change.
In `@apps/docs/content/docs/cli/v2.mdx`:
- Line 30: Replace the phrase "are able to setup tailwindcss and shadcn in your
existing project" with the simpler and grammatically correct "can set up
tailwindcss and shadcn in your existing project" in the sentence that begins
"Unfortunately, there is not an easy path..." so the wording is concise and uses
the correct verb form ("set up" vs "setup").
In `@apps/docs/next.config.ts`:
- Line 24: Add a compatibility redirect for legacy /registry/* URLs by updating
the redirects array returned in the redirects() function (the return currently
containing { source: "/docs", destination: "/docs/cli", permanent: false }).
Insert another redirect entry like { source: "/registry/:path*", destination:
"/docs/cli", permanent: false } before or alongside the existing rule so old
/registry links fall back to /docs/cli while remaining non-permanent.
In `@apps/docs/tests/smoke.test.ts`:
- Around line 3-6: The current tautological assertion inside describe("docs") /
it("has a smoke test") should be replaced with a real behavior check: import the
docs routing/metadata helper (e.g., a function like getDocsRoutes or
docsMetadata) or the app router, invoke it in the test and assert the expected
invariant introduced by this PR (for example, that a specific route path or
metadata key for the new "adding-components" page exists). Update the test body
to call that helper and replace expect(true).toBe(true) with an assertion such
as expect(routes).toContain("/adding-components") or
expect(metadata).toHaveProperty("adding-components"), so the test validates
actual docs state rather than a tautology.
In `@packages/cli/src/cli/add/index.ts`:
- Around line 6-12: Remove the unused noInstall option from the runAdd function
signature: update the options type from { noInstall?: boolean; target?: string }
to only include target, and ensure calls to runAdd (and any callers/types)
expect options?: { target?: string } so that runAdd passes only options?.target
into runAddAddonAction; reference the runAdd function and runAddAddonAction to
locate and update the signature and related types/usages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 66ad6cc2-4631-4965-b1e3-cff0ecee3120
📒 Files selected for processing (47)
.changeset/strong-rings-hide.mdapps/docs/content/docs/cli/guides/adding-components/index.mdxapps/docs/content/docs/cli/guides/adding-components/meta.jsonapps/docs/content/docs/cli/guides/adding-components/templates.mdxapps/docs/content/docs/cli/guides/auth/add-on.mdxapps/docs/content/docs/cli/guides/auth/clerk.mdxapps/docs/content/docs/cli/registry/index.mdxapps/docs/content/docs/cli/v2.mdxapps/docs/content/docs/meta.jsonapps/docs/content/docs/templates/index.mdxapps/docs/content/docs/templates/meta.jsonapps/docs/next.config.tsapps/docs/package.jsonapps/docs/scripts/bundle-registry-templates.jsapps/docs/src/app/(home)/page.tsxapps/docs/src/app/docs/templates/[...slug]/page.tsxapps/docs/src/app/docs/templates/category-config.tsapps/docs/src/app/docs/templates/layout.tsxapps/docs/src/app/docs/templates/page.tsxapps/docs/src/app/docs/templates/templates-client.tsxapps/docs/src/app/llms.txt/route.tsapps/docs/src/app/llms/[package]/route.tsapps/docs/src/app/r/[[...name]]/registry.tsapps/docs/src/app/r/[[...name]]/route.tsapps/docs/src/lib/templates.tsapps/docs/tests/smoke.test.tsapps/docs/tests/utils.manifest.test.tsapps/docs/tsconfig.jsonapps/docs/turbo.jsonpackages/cli/package.jsonpackages/cli/src/cli/add/auth.tspackages/cli/src/cli/add/index.tspackages/cli/src/cli/add/registry/getOptions.tspackages/cli/src/cli/add/registry/http.tspackages/cli/src/cli/add/registry/install.tspackages/cli/src/cli/add/registry/listItems.tspackages/cli/src/cli/add/registry/postInstall/handlebars.tspackages/cli/src/cli/add/registry/postInstall/index.tspackages/cli/src/cli/add/registry/postInstall/package-script.tspackages/cli/src/cli/add/registry/postInstall/wrap-provider.tspackages/cli/src/cli/add/registry/preflight.tspackages/cli/src/consts.tspackages/cli/src/generators/auth.tspackages/cli/src/helpers/shadcn-cli.tspackages/cli/src/index.tspackages/cli/src/installers/envVars.tspackages/cli/tests/cli.test.ts
💤 Files with no reviewable changes (27)
- apps/docs/content/docs/templates/index.mdx
- apps/docs/content/docs/templates/meta.json
- packages/cli/package.json
- apps/docs/src/app/llms.txt/route.ts
- packages/cli/src/cli/add/registry/listItems.ts
- apps/docs/content/docs/cli/registry/index.mdx
- apps/docs/src/app/r/[[...name]]/route.ts
- packages/cli/src/consts.ts
- apps/docs/src/app/docs/templates/page.tsx
- packages/cli/src/cli/add/registry/preflight.ts
- apps/docs/tests/utils.manifest.test.ts
- packages/cli/src/cli/add/registry/http.ts
- packages/cli/src/cli/add/registry/postInstall/package-script.ts
- packages/cli/src/cli/add/registry/postInstall/index.ts
- apps/docs/content/docs/cli/guides/adding-components/templates.mdx
- apps/docs/scripts/bundle-registry-templates.js
- apps/docs/src/app/docs/templates/category-config.ts
- packages/cli/src/cli/add/registry/install.ts
- packages/cli/src/cli/add/registry/getOptions.ts
- apps/docs/src/app/docs/templates/templates-client.tsx
- packages/cli/src/helpers/shadcn-cli.ts
- apps/docs/src/app/docs/templates/layout.tsx
- apps/docs/src/lib/templates.ts
- apps/docs/src/app/docs/templates/[...slug]/page.tsx
- apps/docs/src/app/r/[[...name]]/registry.ts
- packages/cli/src/cli/add/registry/postInstall/handlebars.ts
- packages/cli/src/cli/add/registry/postInstall/wrap-provider.ts
| ### Alternative Authentication Methods | ||
|
|
||
| At this time, FileMaker Add-on Auth will only setup email/password authentication for you, but any other authentication methods can be added by modifying the code yourself. Guides for integrating OAuth providers, passkeys (WebAuthn), two-factor, and more are available in the [Lucia documentation](https://lucia-auth.com/). Just keep in mind that you'll likely need to add more fields to the tables and layouts as mentioned above to support those additional features. No newline at end of file | ||
| At this time, FileMaker Add-on Auth will only setup email/password authentication for you, but any other authentication methods can be added by modifying the code yourself. Guides for integrating OAuth providers, passkeys (WebAuthn), two-factor, and more are available in the [Lucia documentation](https://lucia-auth.com/). Just keep in mind that you'll likely need to add more fields to the tables and layouts as mentioned above to support those additional features. |
There was a problem hiding this comment.
Fix wording typo: use “set up” (verb), not “setup”.
This is user-facing docs copy, so it’s worth tightening.
✏️ Proposed copy fix
-At this time, FileMaker Add-on Auth will only setup email/password authentication for you, but any other authentication methods can be added by modifying the code yourself. Guides for integrating OAuth providers, passkeys (WebAuthn), two-factor, and more are available in the [Lucia documentation](https://lucia-auth.com/). Just keep in mind that you'll likely need to add more fields to the tables and layouts as mentioned above to support those additional features.
+At this time, FileMaker Add-on Auth will only set up email/password authentication for you, but any other authentication methods can be added by modifying the code yourself. Guides for integrating OAuth providers, passkeys (WebAuthn), two-factor, and more are available in the [Lucia documentation](https://lucia-auth.com/). Just keep in mind that you'll likely need to add more fields to the tables and layouts as mentioned above to support those additional features.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| At this time, FileMaker Add-on Auth will only setup email/password authentication for you, but any other authentication methods can be added by modifying the code yourself. Guides for integrating OAuth providers, passkeys (WebAuthn), two-factor, and more are available in the [Lucia documentation](https://lucia-auth.com/). Just keep in mind that you'll likely need to add more fields to the tables and layouts as mentioned above to support those additional features. | |
| At this time, FileMaker Add-on Auth will only set up email/password authentication for you, but any other authentication methods can be added by modifying the code yourself. Guides for integrating OAuth providers, passkeys (WebAuthn), two-factor, and more are available in the [Lucia documentation](https://lucia-auth.com/). Just keep in mind that you'll likely need to add more fields to the tables and layouts as mentioned above to support those additional features. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/docs/content/docs/cli/guides/auth/add-on.mdx` at line 97, Fix the
wording typo in the sentence that currently reads "FileMaker Add-on Auth will
only setup email/password authentication for you" by changing "setup" to the
verb form "set up"; update the copy in the doc content (the sentence containing
"FileMaker Add-on Auth will only setup email/password authentication for you,
but any other...") so it reads "FileMaker Add-on Auth will only set up
email/password authentication for you..." and leave the rest unchanged.
Comment on lines
95
to
97
| if (settings.ui === "shadcn") { | ||
| throw new Error("Shadcn projects should add auth using the template registry"); | ||
| throw new Error("`proofkit add auth` is no longer supported for shadcn projects"); | ||
| } |
There was a problem hiding this comment.
Unify the shadcn unsupported message across both auth paths.
Line 96 now says “no longer supported”, but Line 18 still says “not yet supported”. Please keep one shared message to avoid inconsistent UX.
Proposed refactor
+const SHADCN_AUTH_UNSUPPORTED_MESSAGE =
+ "`proofkit add auth` is no longer supported for shadcn projects";
+
export async function runAddAuthAction() {
const settings = getSettings();
if (settings.appType !== "browser") {
return cancel("Auth is not supported for your app type.");
}
if (settings.ui === "shadcn") {
- return cancel("Adding auth is not yet supported for shadcn-based projects.");
+ return cancel(SHADCN_AUTH_UNSUPPORTED_MESSAGE);
}
@@
.action(async () => {
const settings = getSettings();
if (settings.ui === "shadcn") {
- throw new Error("`proofkit add auth` is no longer supported for shadcn projects");
+ throw new Error(SHADCN_AUTH_UNSUPPORTED_MESSAGE);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/cli/add/auth.ts` around lines 95 - 97, There are two
inconsistent shadcn messages in this module; unify them by extracting a single
constant (e.g., SHADCN_UNSUPPORTED_MESSAGE) and use it wherever the code checks
settings.ui === "shadcn" and throws (replace the throw new Error("...") in the
auth flow that currently uses "no longer supported" and the other location that
says "not yet supported" to reference this constant); update the throw sites to
throw new Error(SHADCN_UNSUPPORTED_MESSAGE) so both auth paths show identical
text.
@proofkit/better-auth
@proofkit/cli
create-proofkit
@proofkit/fmdapi
@proofkit/fmodata
@proofkit/typegen
@proofkit/webviewer
commit: |
- delete obsolete CLI guide - update getting-started next steps - drop guide from CLI nav
Open
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
1 participant
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.
Summary
proofkit addand limit it to supported ProofKit add-ons.Testing
pnpm run cinot runSummary by CodeRabbit
Documentation
Refactor
Tests