latticexyz / mud

MUD is a framework for building autonomous worlds
https://mud.dev
MIT License
753 stars 187 forks source link

templates: figure out workspaces and/or husky+eslint issues #643

Open holic opened 1 year ago

holic commented 1 year ago

Our monorepo is set up such that we have one root pnpm workspace for all our packages, and a top-level templates directory, each with its own pnpm workspace.

Because each template has its own eslint dependencies and config, husky has a hard time knowing which eslint to use when running on files contained within templates. Even with versions aligned, you get an error like:

✔ Preparing lint-staged...
❯ Running tasks for staged files...
  ❯ package.json — 8 files
    ❯ *.ts — 2 files
      ✖ eslint --cache --fix [FAILED]
    ✔ *.{ts,css,md,sol} — 2 files
↓ Skipped because of errors from tasks. [SKIPPED]
✔ Reverting to original state because of errors...
✔ Cleaning up temporary files...

✖ eslint --cache --fix:

Oops! Something went wrong! :(

ESLint: 8.29.0

ESLint couldn't determine the plugin "@typescript-eslint" uniquely.

- /Users/kevin/Projects/latticexyz/mud/templates/minimal/node_modules/.pnpm/@typescript-eslint+eslint-plugin@5.46.1_@typescript-eslint+parser@5.46.1_eslint@8.29.0_typescript@4.9.5/node_modules/@typescript-eslint/eslint-plugin/dist/index.js (loaded in "templates/minimal/packages/client/.eslintrc")
- /Users/kevin/Projects/latticexyz/mud/node_modules/.pnpm/@typescript-eslint+eslint-plugin@5.46.1_@typescript-eslint+parser@5.46.1_eslint@8.29.0_typescript@4.9.5/node_modules/@typescript-eslint/eslint-plugin/dist/index.js (loaded in ".eslintrc")

Please remove the "plugins" setting from either config or remove either plugin installation.

If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.

I thought I could work around this by moving templates into the root workspace, but that has its own issues because pnpm has a hard time with a workspace-within-a-workspace and running commands within a template (i.e. pnpm run build).

 ERR_PNPM_RECURSIVE_RUN_FIRST_FAIL  contracts@0.0.0 build: `mud types`
Exit status 1
 WARN   Local package.json exists, but node_modules missing, did you mean to install?

I think this is because it looks for dependencies at the nearest workspace, but they exist at the root workspace. I'm not sure if there are pnpm settings to work around this, but it feels kinda unsupported.

A potential workaround is remove pnpm-workspace.yaml from each template and add them back in when we copy these directories for use in create-mud, but that seems tricky to maintain and ensure things don't break.

I unblocked myself on the specific lint issue above by committing with --no-verify.

Not in love with any of these options, but posting here to brainstorm alternatives.

holic commented 1 year ago

cc @roninjin10 @alvrs

roninjin10 commented 1 year ago

my 2c is templates shouldn't be opinionated on what linter someone uses and we could just remove the linter from them. Or just make them prettier with no config.

Only exception would be if we had a mud specific lint rule

holic commented 1 year ago

I personally prefer the "linting comes configured out of the box" like Next.js etc. do.

I think we'd still have the same issues if we e.g. let prettier do the "linting"?

Removing linting from templates is certainly the easiest option but my least favorite.

roninjin10 commented 1 year ago

I’m not familiar with lint-staged but can we have it ignore the workspaces and then run individual workspaces seperately