-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Update AI Rules & Docs #431
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@vincanger may be a good idea to include the project directory structure and where certain files should go. If the LLM doesn't know the opensaas structure it will write files where it thinks they should go; this will undoubtedly lead to problems like files/folders being created outside of the app dir or changing files which will lead to a lot of issues with rebasing with upstream. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall lgtm; but, probably could be enhanced in future PRs with more context for better predictability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is some made up Wasp syntax in these rules, I'd advise going through them carefully and making sure all the listed code examples are valid Wasp code.
- **Build/Runtime Errors:** | ||
- Check import paths carefully (Wasp vs. relative vs. `@src/` rules, see [2-project-conventions.mdc](mdc:.cursor/rules/2-project-conventions.mdc) ). | ||
- Ensure all dependencies are installed (`npm install`). | ||
- Check the Wasp server console and the browser's developer console for specific error messages. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should have newlines at the end of files - you get nicer diffs
middleware checkAdmin { | ||
fn: import { checkAdminMiddleware } from "@src/server/middleware/auth.js" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we ever use middleware to check if someone is an admin? Will this example steer Cursur in the wrong direction?
page AdminDashboardPage { | ||
component: import { AdminDashboard } from "@src/features/admin/AdminDashboardPage.tsx", | ||
auth: true, // Ensure user is logged in first | ||
middlewares: [checkAdmin] // Apply custom admin check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
middlewares
doesn't exist as a option of the page
declaration :D I see someone has been vibe coding the vibe coding instructions.
fn: import { handleAdminAction } from "@src/server/apis/admin.js", | ||
httpRoute: (POST, "/api/admin/action"), | ||
auth: true, | ||
middlewares: [checkAdmin] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't exist in Wasp as well, the proper key is middlewareConfigFn
.
- [Wasp Docs - LLMs.txt](https://wasp.sh/llms.txt) - Links to the raw text docs. | ||
- [Wasp Docs - LLMs-full.txt](https://wasp.sh/llms-full.txt) - Complete docs as one text file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose we'll merge this first: wasp-lang/wasp#2772
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah that was my assumption.
## Wasp Auth Setup (`@main.wasp`) | ||
|
||
- Wasp provides built-in authentication with minimal configuration in [main.wasp](mdc:main.wasp). | ||
- See the general [Auth overview and available methods here](mdc:https:/raw.githubusercontent.com/wasp-lang/wasp/refs/heads/release/web/versioned_docs/version-0.16.0/auth/overview.md). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to link to a versioned docs version of the overview.md
file? I saw that you linked to wasp.sh
deployed docs in other places.
} | ||
|
||
// Define the routes needed by email auth methods | ||
route EmailVerificationRoute { path: "/auth/verify-email/:token", to: EmailVerificationPage } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The route should not have :token
as param in it, the token is passed in as a query value i.e. ?token=...
. I believe this code will fail.
route EmailVerificationRoute { path: "/auth/verify-email/:token", to: EmailVerificationPage } | ||
page EmailVerificationPage { component: import { EmailVerification } from "@src/features/auth/EmailVerificationPage.tsx" } | ||
|
||
route PasswordResetRoute { path: "/auth/reset-password/:token", to: PasswordResetPage } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above
```prisma | ||
model User { | ||
id Int @id @default(autoincrement()) | ||
email String? @unique // Add if needed frequently |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can reference the userSignupFields
hook and how they can set the email
on signup? You'll know better which level of details is okay :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah good idea. I added a note about that in the rule even though its present in the open saas codebase because we will probably use these rules for other projects/templates.
} | ||
``` | ||
|
||
</rewritten_file> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leftover </rewritten_file>
and also, let's add a newline
Description
This adds improved
.cursor/rules
in the form of multiple .mdc files, as well as a docs section on how to use AI with Open SaaS. Fixes #428Contributor Checklist