prettier icon indicating copy to clipboard operation
prettier copied to clipboard

feat(sh): add pragma support

Open Kenneth-Sills opened this issue 1 year ago β€’ 4 comments

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.

Kenneth-Sills avatar Jul 25 '24 15:07 Kenneth-Sills

πŸ¦‹ Changeset detected

Latest commit: dbcb5a82947a23caa26fe8f8a6955073bdeafdb0

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

changeset-bot[bot] avatar Jul 25 '24 15:07 changeset-bot[bot]

@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/@[email protected][email protected][email protected]/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/@[email protected][email protected][email protected]/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/@[email protected][email protected][email protected]/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!

Kenneth-Sills avatar Jul 25 '24 15:07 Kenneth-Sills

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.

codesandbox-ci[bot] avatar Aug 29 '24 06:08 codesandbox-ci[bot]

@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!

Kenneth-Sills avatar Aug 29 '24 06:08 Kenneth-Sills

New and removed dependencies detected. Learn more about Socket for GitHub β†—οΈŽ

Package New capabilities Transitives Size Publisher
npm/@types/[email protected] None 0 10.8 kB types
npm/[email protected] None 0 228 kB fatfisz

View full reportβ†—οΈŽ

socket-security[bot] avatar Feb 20 '25 07:02 socket-security[bot]

Sorry I didn't notice this PR for a long time, seems great to have.

JounQin avatar Feb 20 '25 07:02 JounQin