search

0xFableOrg / 0xFable

A fully on-chain trading card game. There will be elves, wizards & shit. Drama and broken friendships also.
https://twitter.com/0xFableGame
BSD 3-Clause Clear License
106 stars 39 forks source link

Add package manager to package.json #45

Closed nezouse closed 1 year ago

nezouse commented 1 year ago

Should be easier to make sure you use correct version of pnpm now.

norswap commented 1 year ago

What's the procedure to update this? Say I update pnpm and do make install (pnpm install -r), will that automatically update both the lockfile and the version in package.json?

nezouse commented 1 year ago

Lockfile will be updated automatically when you install new deps, but version in package.json has to be managed manually

norswap commented 1 year ago

Is there a way to modify make install such that in addition to updating the lockfile, it also updates the packageManager property?

If that's too hard, we could add corepack enable to make setup then change all pnpm commands in the makefile to use corepack pnpm instead?

nezouse commented 1 year ago

Not sure why would you want to update the packageManager field. It's sets which version of pnpm you want to use and i don't think it's something that should be updated by accident. Tbh i added it more as an escape hatch if pnpm is not working if someone has a wrong version installed. Also it's hard to say about corepack enable as right now this command throws for me, so not sure if you can run it multiple times.

norswap commented 1 year ago

The scenario I'm thinking of is that right now if I (1) update pnpm, (2) run make install then push, then the version is not correct anymore (mismatch between pnpm-lock.json and packageManager & even someone using corepack will get an error.

Unless pnpm itself checks packageManager & prevents this from happening?

norswap commented 1 year ago

Ok, so I think this is good enough for now. Thinking of it, there is no trivial solution anyway, since the lock file version doesn't update with every version of pnpm. We'll just have to be vigilant to upgrade the statement in the package file every time the lock-file version changes.

I've added warnings to the Makefile and that'll have to do!

Many thanks for the PR!

norswap commented 1 year ago

I completely missed that this also was setting up Vite. This was not fully integrated in the build flow & needs further review, so I've reverted on master.

However, feel free to open a new PR for setting up Vite!