Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(sh): add pragma support #378

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kenneth-Sills
Copy link

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.

Copy link

changeset-bot bot commented Jul 25, 2024

🦋 Changeset detected

Latest commit: c91daca

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
Copy link
Author

@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 Kenneth-Sills force-pushed the kesills-sh-pragma-support branch from bc1493c to 6a3e2a9 Compare August 29, 2024 06:47
Copy link

codesandbox-ci bot commented Aug 29, 2024

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 Kenneth-Sills force-pushed the kesills-sh-pragma-support branch from 6a3e2a9 to c213e86 Compare August 29, 2024 06:49
@Kenneth-Sills Kenneth-Sills force-pushed the kesills-sh-pragma-support branch from c213e86 to c91daca Compare August 29, 2024 06:51
@Kenneth-Sills
Copy link
Author

Kenneth-Sills commented Aug 29, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[prettier-plugin-sh] requirePragma Option Not Respected
1 participant