polkadot-api / polkadot-api

Polkadot-API is a meticulously crafted suite of libraries, each designed to be composable, modular, and aligned with a "light-client first" philosophy.
https://papi.how
MIT License
97 stars 24 forks source link

Make `runInstall` in the CLI configurable to be turned off #789

Open ryanleecode opened 1 month ago

ryanleecode commented 1 month ago

https://github.com/polkadot-api/polkadot-api/blob/de3f57103460478322d29fb5c7c44b722f5664fb/packages/cli/src/commands/generate.ts#L85

I have a monorepo and i isolated the descriptors so i can used turbo to cache the build. I use my normal setup with the build script in the prepare script but it runs in an infinite loop b/c the CLI always calls pnpm install when its done. Note, this package will never be published, its just sits in a private monorepo; the usecase is to leverage caching so papi does not need to be run on every pnpm i.

Please do not close this issue by stating "i'm shooting myself in the foot" or my usecase is "fictitious" before a counter argument can be made. thanks.

turbo.json

{
  "$schema": "https://turbo.build/schema.json",
  "extends": ["//"],
  "tasks": {
    "build": {
      "dependsOn": ["^build"],
      "inputs": [".papi/metadata/*", ".papi/polkadot-api.json"],
      "outputs": [".papi/descriptors"]
    }
  }
}

package.json

{
  "name": "@paritytech/klit-descriptors",
  "private": true,
  "version": "0.0.1",
  "author": "Ryan Lee <ryan@parity.io>",
  "type": "module",
  "main": "./.papi/descriptors/dist/index.js",
  "types": "./.papi/descriptors/dist/index.d.ts",
  "module": "./.papi/descriptors/dist/index.mjs",
  "exports": {
    "./package.json": "./package.json",
    ".": {
      "import": {
        "types": "./.papi/descriptors/dist/index.d.ts",
        "default": "./.papi/descriptors/dist/index.mjs"
      },
      "require": {
        "types": "./.papi/descriptors/dist/index.d.ts",
        "default": "./.papi/descriptors/dist/index.js"
      }
    }
  },
  "files": [
    ".papi"
  ],
  "scripts": {
    "prepare": "pnpm turbo build",
    "clean": "rimraf .papi/descriptors",
    "build": "pnpm run clean && papi"
  },
  "dependencies": {
    "@polkadot-api/descriptors": "file:.papi/descriptors"
  },
  "devDependencies": {
    "polkadot-api": "catalog:",
    "typescript": "catalog:"
  },
  "peerDependencies": {
    "polkadot-api": "catalog:"
  }
}
voliva commented 1 month ago

I need to take a deeper look. In theory, we try to skip running "generate" if we detect it's coming from a previous generate.

I'm thinking maybe this could be improved also by not running npm install (or equivalent) if we detect the descriptors haven't changed.

Adding an option is also possible, but I rather first see if we can handle this internally rather than making the public API larger.

voliva commented 1 month ago

Problem was that turbo doesn't pass environment variables down to scripts, so our flag to detect this loop failed. A possible workaround is to change the prepare script from pnpm turbo build to just pnpm build, as that only effects that specific package.

I've added a fix in #805 which skips running install if the descriptors haven't changed, as it's not necessary and should avoid similar cases like these, but for that to work you should not clean the descriptors package.json. We don't put it in the .gitignore because it's important, and it specifies the actual version of the descriptors package.

The descriptors/dist folder is safe to be cleaned (and is also .gitignored), but we actually remove it and create it from scratch before every papi generate, so you shouldn't need to clean it either.

Edit - the fixed I've added doesn't work for regular repos on first install, looking at alternatives now. Meanwhile you have the workaround of changing "pnpm turbo build" for "pnpm build" on prepare.

ryanleecode commented 1 month ago

I just did pnpm patch and commented it out. but just changing it to pnpm build from pnpm turbo build defeats the whole purpose since its not going to cache w/o turbo.

ryanleecode commented 1 week ago

Prob good to also add a TODO for docs on how to use this feature. @voliva

carlosala commented 1 week ago

Good point, I'll handle that👍 Nevertheless, it won't be released until papi 1.8, and we don't have an ETA. After it I'll close this issue. Thanks Ryan!