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

Resolve issues when installing the Prisma client in a PNPM workspace using Nuxt layers #30

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Dobefu
Copy link
Contributor

@Dobefu Dobefu commented Aug 5, 2024

This PR will fix #29

Copy link

vercel bot commented Aug 5, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nuxt-prisma ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 30, 2024 2:33pm

@Dobefu
Copy link
Contributor Author

Dobefu commented Aug 5, 2024

Please note: on this version of the PR, installation will happen in the wrong Nuxt layer. A workaround for now is to manually install the client and set the installPrismaClient option to false.
I do not have much time to look into this further for a couple of days, so I hope this is a helpful start at least.

@Dobefu
Copy link
Contributor Author

Dobefu commented Aug 13, 2024

I'm back!

@ankur-arch I would like to add two configuration options, but I'd like your thoughts on it:

I think especially the second option would benefit a lot of people, since I don't see a way that we can detect it in the code in a reliable and performant manner.

@ankur-arch
Copy link
Contributor

Thanks @Dobefu! I'll check on this the next week as I'm on limited availability this week.

@husayt
Copy link

husayt commented Nov 13, 2024

@ankur-arch will be really nice if this could be checked. Many users of nuxt-prisma are using layers and if already there is a solution to this issue, why not to use it.

@husayt
Copy link

husayt commented Nov 14, 2024

@ankur-arch will be really nice if this could be checked. Many users of nuxt-prisma are using layers and if already there is a solution to this issue, why not to use it.

Just to check if the issue I had was related to this and not something else, I switched to npm and it worked. So it was definetely caused by lack of pnpm support. Switching to npm can be a workaround for now

@ankur-arch
Copy link
Contributor

Hey @Dobefu 👋,

This has been in draft for a while, so I wanted to check in to see if you’re planning to get to this. Thanks for your contributions so far—they’ve been really valuable 🔥 !

Let me know if you’re planning to pick this up; otherwise, I’ll move forward with the updates to ensure compatibility with other package managers and Nuxt layers, as I’d like to proceed soon.

Looking forward to hearing from you and possibly collaborating again 😄 !

@Dobefu
Copy link
Contributor Author

Dobefu commented Nov 28, 2024

Hi @ankur-arch!

I'm certainly open to collaborate again!

To be quite honest, I was waiting for your thoughts on #30 (comment) and then proceeded to... kind of forget to follow up on it 😅. My apologies. What are your thoughts on the configuration options in my previous comment? If they're alright to add, I can look into it this weekend.

@Dobefu
Copy link
Contributor Author

Dobefu commented Nov 30, 2024

Alrighty, I've fixed the merge conflicts.
I've tested it, and I believe that the PR is pretty much ready to be reviewed.
I'll just assume that the two extra options are fine 🙂

@Dobefu Dobefu marked this pull request as ready for review November 30, 2024 15:08
@Dobefu
Copy link
Contributor Author

Dobefu commented Dec 15, 2024

Hi @ankur-arch,

Have you had time to look into this, yet?
A CI task has failed, but as far as I can see, this might be because of permissions on my end. It shouldn't be related to my changes at all, unless I'm mistaken.

@ankur-arch ankur-arch self-requested a review December 17, 2024 08:20
@ankur-arch ankur-arch self-assigned this Dec 17, 2024
@jharrell jharrell self-assigned this Dec 19, 2024
@ankur-arch
Copy link
Contributor

Hey @Dobefu 👋 ,

I apologize for my recent absence; I've been busy with other commitments. It's great to have you back, and I really appreciate you taking the time to work on this PR and resolve the merge conflicts.

Have you had time to look into this, yet?
I noticed one of the CI tasks failed. From what I can tell, this seems to be related to permission issues on my end rather than any changes in the PR.

Since this PR is from a fork, GitHub won't allow the preview package to be published through the workflow, so I'll need to test it manually.

I'll just assume that the two extra options are fine 🙂

The options look good to me!

I'll test the PR tomorrow and then proceed with merging it. I'll also review any accompanying documentation changes to ensure everything is consistent.

@ankur-arch
Copy link
Contributor

Hey @Dobefu 👋,

I gave the PR a try and still have a few tests left to go. Sorry for the delay—I'll wrap it up on Monday! 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Package manager cannot get resolved when using Nuxt Layers
4 participants