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

Removed duplicated cn function declaration from lib dir #74065

Merged

Conversation

damiensedgwick
Copy link
Contributor

@damiensedgwick damiensedgwick commented Dec 18, 2024

This pull request removes duplication of a utility function within the with-supabase example as mentioned in the following issue #73942 - I removed the duplicated code from the utils directory as when shadcn installs a new component, that component will look for the helper function in lib/utils.ts and forcing users to update every component after installation or modify any config files for shadcn seems like a worse DX.

@ijjk ijjk added the examples Issue was opened via the examples template. label Dec 18, 2024
@ijjk
Copy link
Member

ijjk commented Dec 18, 2024

Allow CI Workflow Run

  • approve CI run for commit: cc16cfb

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

@ijjk
Copy link
Member

ijjk commented Dec 18, 2024

Allow CI Workflow Run

  • approve CI run for commit: 1a522a1

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

@damiensedgwick
Copy link
Contributor Author

@samcx Hey, here is the pr as discussed. it feels a little wrong leaving an empty lib directory but given the approach of using a Data Access Layer going forward, I think it makes sense. I can always add a placeholder file or README.md in there if we feel like it would be beneficial?

@samcx samcx self-requested a review December 18, 2024 17:11
@samcx
Copy link
Member

samcx commented Dec 18, 2024

@damiensedgwick Can you make sure to update the path imports? Also seeing an issue with the Tailwind CSS.

You can test this locally with pnpm next-with-deps examples/with-supabase or manually start it up locally with changes via Create Next App.

@damiensedgwick
Copy link
Contributor Author

damiensedgwick commented Dec 18, 2024

@damiensedgwick Can you make sure to update the path imports? Also seeing an issue with the Tailwind CSS.

You can test this locally with pnpm next-with-deps examples/with-supabase or manually start it up locally with changes via Create Next App.

Fixed the issue with component imports. I actually realised that I removed the wrong instance at first so that has been corrected.

Regarding tailwind, this is a weird error because I have not touched anything Tailwind related but also, the error I am getting

Syntax error: next.js/examples/with-supabase/app/globals.css The border-border class does not exist. If border-border is a custom class, make sure it is defined within a @layer directive.

Does not make sense because the class is within a layer directive as we can see here:

@layer base {
  * {
    @apply border-border;
  }
  body {
    @apply bg-background text-foreground;
  }
}

So I am not entirely sure what you would like me to do here. This is exactly how the css file is setup within every project using shad that I have seen.

Copy link
Member

@samcx samcx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for submitting a PR!

:lgtm:

@samcx samcx enabled auto-merge (squash) December 18, 2024 18:37
@samcx samcx merged commit a216207 into vercel:canary Dec 18, 2024
39 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
examples Issue was opened via the examples template. locked
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Supabase example has duplicate utils for cn.ts
3 participants