Breaks in pnpm environment

[waspeer]

Hi!

Thanks for making this package! It's fantastic :)
I'm using pnpm as my package manager and I have been getting errors after building Sanity studio. To reproduce:

pnpm create sanity@latest

# project: test
# dataset: test
# output: test
# template: no schemas
# typescript: yes

cd test
pnpm install @sanity-typed/types

// sanity.config.ts
import {defineConfig} from '@sanity-typed/types' // <-- change this line to import from sanity-typed
// ...

pnpm run build
pnpm run start

The error I'm getting:

Uncaught error: Pe.default.div is not a function
http://localhost:3333/static/sanity-9dde895e.js:4670:40453
TypeError: Pe.default.div is not a function
    at http://localhost:3333/static/sanity-9dde895e.js:4670:40453
    at http://localhost:3333/static/sanity-9dde895e.js:7423:54634

This error resolves when I change this line back:

// sanity.config.ts
import {defineConfig} from 'sanity' // <-- change this line to import from sanity
// ...

I wasn't able to create a StackBlitz or something similar doing these steps, so I'm not sure how this reproduces on other machines. I'm using a Macbook Pro with Ventura 13.4

[saiichihashimoto]

So I've never used pnpm and I wasn't really planning on supporting it, although it's nice to identify this issue twice as a pnpm specific issue. I have no idea what's happening. My immediate guess is that the default.div error has something to do with styled-components, which I think that sanity uses and, since sanity is a peer dependency, the issue has something to do with how pnpm handles or doesn't handle peer dependencies. Maybe you can help solve this.

Even if I were to support this, I'm not sure how to maintain it. It seems difficult to get CI to setup a repo with each package as a dependency, attempt npm/yarn/pnpm install, build, and determine that running also works (because in this case, it's actually running the studio that identifies the issue). Without this full pipeline, the chances that I re-break pnpm without knowing it are extremely high.

[shixish]

Got snagged by this as well 😢. It'd be nice to add a warning to the readme page saying it'll break if you use pnpm. Sanity offers pnpm as a supported option when you spin up a new project so it's likely that more people are going to stumble here. It's also tricky to figure out what's going on since it works in dev mode but breaks in prod mode. 🤦

[waspeer]

Yeah its a tricky issue. I did not have the time to dig into it yet, but my suspicion is that it has something to do with the portabletext editor dependency, since that is the only one in this package that depends on styled components. I hope to be able to look into this later, but if you find out something I'm curious to hear your findings.

[mrr11k]

Also stumbled on it. 😕 Will also try to dig into it soon.
Good to know that the portabletext editor could be the issue. I will report here if I find something.

[saiichihashimoto]

Anyone have any ideas of how to test this in CI, before we tackle solving it? I took a first swing at it, but ended up with a solution on how to build this whole repo using pnpm, which is not what I want. I want to figure out if, when another repo uses @sanity-typed/types (as an example, we can run tests for each package) and pnpm, that we don't get build errors. I'm also concerned that, even with the reported errors, it's not building the sanity studio that's failing but running it, which seems even worse to test. I'm convinced this isn't just difficult to test, but flaky to maintain.

Basically, how do other libraries that don't use pnpm internally validate when an application using the library does use pnpm?

[saiichihashimoto]

(separately it looks like there's some other issue involving portable text and pnpm which, if we had this CI setup, would hopefully identify that into a separate issue.)

I'm sure pnpm has it's own pros and cons, but the fact that it doesn't work with libraries that do work with npm is pretty terrible. Pretty difficult to argue it's a drop-in replacement for npm when arbitrary libraries just don't work with it.

[waspeer]

Hm yes, it sounds like this will be a very tricky issue to test. Not impossible, but I'm wondering if it's worth the resources and I agree it sounds flaky as hell. Locally I remember testing this by changing stuff, re-running a one line command that built and ran the production app (pnpm run build && pnpm run preview) and then refreshing the browser to see if it worked (probably similar to how you are doing it as well). Also, I remember that Astro had issues with pnpm in the early days as well, maybe they have managed to create some tests for that?

Maybe it's good to first figure out what's causing this issue and then considering if that's something worth adding tests for. What do you think? I'm expecting this to be an upstream bug from either the portable text package, possibly some edge case in how the dependencies are declared that pnpm is stricter about. If we're really unlucky maybe even a bug in pnpm itself.

I agree with your frustration about pnpm. Even though I've ran into annoying issues like these a couple of times now, I still prefer using it myself. Additionally I think it's only growing in popularity, so it's probably worth the effort to figure this out.

[saiichihashimoto]

1. I don't think you're disagreeing, but this library will build using npm. So I'd need to test that the completely npm build library works in a pnpm environment, which isn't as simple as switching everything over to pnpm and seeing if things work. For me, that's most of the complexity. ie, supporting pnpm is very different than having this library just use npm. Likely, similar issues might occur with yarn. Reproducing the environment required for this issue is non-trivial locally or in CI. And, if it isn't reproduced in CI, I can't claim support for it.
2. I'd like to isolate this specific issue about portable text so I can test for it. However, it only surfaces when someone is using pnpm, so I can't reproduce the issue without completing point number 1 anyways (it very well might be a pnpm exclusive problem, which it seems to be). And to include tests that recreate the issue will be difficult because the issue doesn't occur currently, only when in a pnpm environment which requires point number 1.
3. In terms of documenting this, I'm also not documenting whether or not it works in yarn for all the same reasons all packages aren't listing if they work in npm, yarn, pnpm, mynextnpmimplementation, etc. They are supposed to support all npmjs.org packages out of the box, as-is. Even if we can repro this properly, it feels like the maintenance burden is on the wrong side of the fence here.

Maybe this will help? I couldn't really sort it out but it might be helpful.