Skip to content
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

enhancement: index.tsx doesn't include the correct cards #190

Closed
anthonylaflamme opened this issue Jul 14, 2022 · 13 comments · Fixed by #381
Closed

enhancement: index.tsx doesn't include the correct cards #190

anthonylaflamme opened this issue Jul 14, 2022 · 13 comments · Fixed by #381
Labels
📌 area: t3-app Relates to the generated T3 App 🔰 good first issue Good for newcomers 🌟 enhancement New feature or request 👨‍👦‍👦free for all Anyone is free to take on this issue and file a PR

Comments

@anthonylaflamme
Copy link

anthonylaflamme commented Jul 14, 2022

Describe the bug
Create t3 app with all possible option will not return the correct page studs seems to have broke in : c436382

To Reproduce
Steps to reproduce the behavior:

  1. Run ct3 app
  2. Enable all possible feature
  3. Run the 3 commands
  4. Go to localhost
  5. See that only 4 cards on the page stud are showing missing prisma and next auth

Expected behavior
Page stud should give out the according cards that were selected when scaffolding (verify all page stud seems like they all use the default component)

Screenshots
If applicable, add screenshots to help explain your problem.

Sorry i'm on phone je

@anthonylaflamme
Copy link
Author

I just started a fix for this will update soon

@anthonylaflamme
Copy link
Author

Let me know if I should continue @nexxeln or if there was other discussion concerning this thank you

@nexxeln
Copy link
Member

nexxeln commented Jul 14, 2022

Let me know if I should continue @nexxeln or if there was other discussion concerning this thank you

Please go ahead! Thank you for reporting this.

@CarlosGomez-dev
Copy link
Contributor

hey @anthonylaflamme I have a PR open #189 fixing the issues introduced in #178, it seems it used an outdated main branch as base so it undid a few previous PRs.

@anthonylaflamme
Copy link
Author

hey @anthonylaflamme I have a PR open #189 fixing the issues introduced in #178, it seems it used an outdated main branch as base so it undid a few previous PRs.

Oh no worries then go ahead ! Thank you

@CarlosGomez-dev
Copy link
Contributor

I do think we have an issue with the home component in general. We don't have enough templates to handle all the possible combinations of the 4 packages we offer, and the more templates we create the harder it is to keep them up to date.

@t3dotgg
Copy link
Member

t3dotgg commented Jul 14, 2022

@CarlosGomez-dev makes a good point. I wonder if there's some way we can "encode" the choices as like an array and then render the right cards for that array?

@xpressivecode
Copy link
Contributor

There could be a missed opportunity in not including the technology options as a prisma model? It would show a good full circle use of primsa, trpc and the new TechnologyProps / component that was recently introduced. Vs right now I think there is an empty "example" model that really doesn't showcase much for new users.

If the tech option was a model that included a short code or slug of sorts, they could be queried depending on the template options that were picked in the findMany used by the trpc query?

const options = [next-auth, tailwind]; // set in home page and passed via trpc query?
tech.findMany({ where: { slug: { in: options } } });

It could also show developers how to seed their database and reduce some of the repetitive copy.

@c-ehrlich
Copy link
Member

There could be a missed opportunity in not including the technology options as a prisma model? It would show a good full circle use of primsa, trpc and the new TechnologyProps / component that was recently introduced. Vs right now I think there is an empty "example" model that really doesn't showcase much for new users.

Not every ct3a includes Prisma.

Another possibility would be to check at runtime using require.resolve('<module-name>'), but this can't check for Tailwind as that's a devDependency.

Maybe the easiest thing is really to just hardcode an array of installed stuff during initialization.

@juliusmarminge
Copy link
Member

I think we can move forward with this without codemods since progress there seems to be a bit stale right now, so if anyone wants to make a neat way to include the correct cards, feel free.

Another approach could be to just include all cards for every template?

@juliusmarminge juliusmarminge added 📌 area: t3-app Relates to the generated T3 App 🔰 good first issue Good for newcomers 👨‍👦‍👦free for all Anyone is free to take on this issue and file a PR and removed on hold labels Aug 26, 2022
@juliusmarminge juliusmarminge changed the title Page stud borken enhancement: index.tsx doesn't include the correct cards Aug 27, 2022
@c-ehrlich
Copy link
Member

Including all cards on every template makes sense imo.

@nexxeln
Copy link
Member

nexxeln commented Aug 30, 2022

Including all cards on every template makes sense imo.

Are you free to make a PR for this? I'm also leaning towards this.

@c-ehrlich
Copy link
Member

Including all cards on every template makes sense imo.

Are you free to make a PR for this? I'm also leaning towards this.

yea sure, will do hopefully today

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📌 area: t3-app Relates to the generated T3 App 🔰 good first issue Good for newcomers 🌟 enhancement New feature or request 👨‍👦‍👦free for all Anyone is free to take on this issue and file a PR
Projects
None yet
7 participants