un-ts / prettier

:package: Opinionated but Incredible Prettier plugins.
https://prettier.vercel.app
MIT License
277 stars 26 forks source link

feat(sh): add pragma support #378

Open Kenneth-Sills opened 3 months ago

Kenneth-Sills commented 3 months ago

Closes #377


The approach I took differs from that described in the related ticket. As I started working and read Prettier's plugin API and implementation, I realized we don't have access to all of the options required to instantiate the parser until after the pragma check has happened. Without that, we couldn't cache or memoize and using the actual Parser would mean every file would need to be parsed twice.

So this takes a regex-based approach to just chomp off the top of a file until it comes across non-comment content, scanning for the pragma along the way.

changeset-bot[bot] commented 3 months ago

🦋 Changeset detected

Latest commit: c91daca5d20f02599d5597c7783e27042c7ca23a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | ------------------ | ----- | | prettier-plugin-sh | Minor |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Kenneth-Sills commented 3 months ago

@JounQin Hey, I'm sorry, I've tried to get that changeset file generated but I'm having trouble. I get through the prompt asking for what release needs to happen and to summarize the changes, but after confirming it crashes with:

🦋  error Error: Cannot find module '/workspaces/un-ts-prettier-plugins/node_modules/.pnpm/@1stg+prettier-config@3.9.2_prettier@3.2.4_svelte@4.2.12/node_modules/prettier-plugin-pkg/lib/index.cjs'
🦋  error     at createEsmNotFoundErr (node:internal/modules/cjs/loader:1055:15)
🦋  error     at finalizeEsmResolution (node:internal/modules/cjs/loader:1048:15)
🦋  error     at resolveExports (node:internal/modules/cjs/loader:556:14)
🦋  error     at Function.Module._findPath (node:internal/modules/cjs/loader:596:31)
🦋  error     at Function.Module._resolveFilename (node:internal/modules/cjs/loader:1014:27)
🦋  error     at Function.Module._load (node:internal/modules/cjs/loader:873:27)
🦋  error     at Module.require (node:internal/modules/cjs/loader:1100:19)
🦋  error     at require (node:internal/modules/cjs/helpers:119:18)
🦋  error     at /workspaces/un-ts-prettier-plugins/node_modules/.pnpm/@1stg+prettier-config@3.9.2_prettier@3.2.4_svelte@4.2.12/node_modules/@1stg/prettier-config/base.js:20:5
🦋  error     at Array.map (<anonymous>) {
🦋  error   code: 'MODULE_NOT_FOUND',
🦋  error   path: '/workspaces/un-ts-prettier-plugins/node_modules/.pnpm/@1stg+prettier-config@3.9.2_prettier@3.2.4_svelte@4.2.12/node_modules/prettier-plugin-pkg/package.json'
🦋  error }

This happens both through npx changeset and pnpm exec changeset. I was on Node 20 during development initially, then switched to Node 16, but it's still happening. Also tried on a separate PC.

I'm not too familiar with the tooling (PNPM and Changeset), so I would appreciate it if you could let me know what I did wrong here!

codesandbox-ci[bot] commented 2 months ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Kenneth-Sills commented 2 months ago

@JounQin Thanks for your patience, this should now be ready for workflows and review!

For posterity, I needed to:

  1. Move common-tags to a dev dependency specific to the package it's being used in. Add the corresponding @types/common-tags type definitions for the build process to use.
  2. Rollback my changes to pnpm-lock.yaml and regenerate it for the changed dependencies with PNPM v8 (I was previously on 9, which made a BC-breaking lockfile version update).
  3. Reformat the new files and satisfy ESLinter on a complaint about while (true) by switching to for (;;).
  4. Finally, to get that changeset generated I just needed to make sure I ran pnpm build before pnpm changeset. The install process on it's own just links/copies things around without topological build ordering, which is why it was failing to locate that module / getting confused on ESM vs CJS imports.

It would probably be a good idea to document (4) somewhere, but I think the changes here are sufficient for this PR and the docs update can come later.

Let me know if you need anything else. Thanks again!