prettier / plugin-pug

Prettier Pug Plugin
https://prettier.github.io/plugin-pug
MIT License
196 stars 44 forks source link

v3.0.0 #458

Closed Shinigami92 closed 1 year ago

Shinigami92 commented 1 year ago

closes #411 closes #456

Shinigami92 commented 1 year ago

@lehni you said you have a project? Could you please checkout the branch and run pnpm run preflight + yarn link and test if everything is OK?

Please also check TypeScript support for the .prettierrc.cjs file as documented

Shinigami92 commented 1 year ago

And here are the rendered changelogs

https://github.com/prettier/plugin-pug/blob/v3/CHANGELOG.md#300

lehni commented 1 year ago

@Shinigami92 I'll check tomorrow, thanks. You probably want to still remove all the // since:…. comments?

fisker commented 1 year ago

https://github.com/prettier/plugin-pug#usage

should add --plugin since Prettier don't auto load plugin anymore. Maybe also worth mention in changelog.

Shinigami92 commented 1 year ago

@Shinigami92 I'll check tomorrow, thanks. You probably want to still remove all the // since:…. comments?

No, I like the info about when a feature was added (without needing to search the git history)

Shinigami92 commented 1 year ago

prettier/plugin-pug#usage

should add --plugin since Prettier don't auto load plugin anymore. Maybe also worth mention in changelog.

-> https://github.com/prettier/plugin-pug/pull/458/files#diff-a9f09e5fd39208124b216a0cac772b60ab660bf177b9ecd1ab87f18006cecdbcR98

Is this info not sufficient enough? I feel like the plugin was never auto-loaded, and I needed to do this step. But that could be because I use pnpm everywhere.

fisker commented 1 year ago

Is this info not sufficient enough?

Missed that.

lehni commented 1 year ago

I feel like the plugin was never auto-loaded, and I needed to do this step. But that could be because I use pnpm everywhere.

Auto-loading worked for me with v2, and it not working anymore def caught me a bit off guard at first when testing v3.

lehni commented 1 year ago

@Shinigami92 I did a link test now, and I'm getting this error here:

[eslint] Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/Users/lehni/Development/Lineto/plugin-pug/dist/logger' imported from /Users/lehni/Development/Lineto/plugin-pug/dist/index.js

When I look at the import statement, I see that it's missing the file extension:

import { logger } from './logger';

Related: https://nodejs.org/api/esm.html#mandatory-file-extensions

lehni commented 1 year ago

This might be our solution too: https://github.com/vitejs/vite/issues/11552#issuecomment-1507927724

Shinigami92 commented 1 year ago

@Shinigami92 I did a link test now, and I'm getting this error here:

[eslint] Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/Users/lehni/Development/Lineto/plugin-pug/dist/logger' imported from /Users/lehni/Development/Lineto/plugin-pug/dist/index.js

When I look at the import statement, I see that it's missing the file extension:

import { logger } from './logger';

Related: nodejs.org/api/esm.html#mandatory-file-extensions

git pull --rebase && pnpm run preflight test again pls 🙂 I used tsup now and there is only the dist/index.js as minified version now

lehni commented 1 year ago

@Shinigami92 I got it running like this:

https://github.com/lehni/plugin-pug/commit/3b2ce18ecda0acfa40cef2824d994ffdab2631eb

Now trying your version

lehni commented 1 year ago

@Shinigami92 great news, everything appears to be working for me now! Even VSCode integration and format on save works as before, it's all a breeze now :)

I also tested the auto-completion in the prettierrc.cjs file:

image
lehni commented 1 year ago

@Shinigami92 I approved it now. Not sure if 3b2ce18ecda0acfa40cef2824d994ffdab2631eb wouldn't give you the better dev experience though if there are bugs / error logs

Shinigami92 commented 1 year ago

I got it running like this: lehni@3b2ce18

I personally don't like having .js suffixes in my TypeScript files

I approved it now. Not sure if 3b2ce18 wouldn't give you the better dev experience though if there are bugs / error logs

We have enabled source maps, so the files are pointing back to the src folder which is also shipped in the package

lehni commented 1 year ago

Then merge away, I'd say 🎉

Shinigami92 commented 1 year ago

Then merge away, I'd say 🎉

Will do when I'm back home 😬

But one last thing, do I need to add the src folder to the exports in package.json? Because I don't the see pug* suggestions the the tooltip in prettier config 🤔

lehni commented 1 year ago

@Shinigami92 my bad, the pug attributes are indeed missing! Let's test this, I'm not sure how this work and what needs to be done to support it. Personally I don't use TS and am not all that excited about auto-completion, so I never really notice when it's not there :)

lehni commented 1 year ago

@Shinigami92 but come to think of it, with yarn link the source folder is still around, and yet I don't see the suggestions.

So this link here resolves, when clicking through it, i get to the editor for the right file:

// @ts-check
/// <reference types="@prettier/plugin-pug/src/prettier" />
image

But it does show an error there, and the auto-completion doesn't works for pug keywords, only for the standard prettier ones.

The error on the type file:

image
Shinigami92 commented 1 year ago

@lehni Could you check a last time if the auto-suggestions now works in the prettierrc? I exported now also src and package.json

  "exports": {
    ".": {
      "types": "./dist/index.d.ts",
      "default": "./dist/index.js"
    },
    "./src/*": {
      "types": "./src/*.d.ts",
      "default": "./src/*.ts"
    },
    "./package.json": "./package.json"
  },
lehni commented 1 year ago

@Shinigami92 I've just tested it, and sadly nothing has changed. I think it's probably easier if I share a test-project for you to replicate this using yarn link ?

Shinigami92 commented 1 year ago

I think it's probably easier if I share a test-project for you to replicate this using yarn link?

No, thx, I have pnpm, not yarn

Okay, I will set me up my own playground and try a bit :thinking:

lehni commented 1 year ago

@Shinigami92 the two are not mutually exclusive :) You can have yarn and npm and pnpm, all coexist in a friendly way.

Shinigami92 commented 1 year ago

@Shinigami92 the two are not mutually exclusive :) You can have yarn and npm and pnpm, all coexist in a friendly way.

Uhm... does yarn have a bug? It works for me :thinking: Did you pull the latest change?

Shinigami92 commented 1 year ago

@lehni I think I will then just make a release and if someone reports a bug with that, I can handle it later on

lehni commented 1 year ago

You know what? It works for me too! I didn't notice before that I also had prettier linked to a local version, maybe something there was messing things up. I'm sorry for the sloppy testing!

image
lehni commented 1 year ago

I just updated our code-base to the released v3, and it all works, auto-completion included. Thank you @Shinigami92 !

lehni commented 1 year ago

@Shinigami92 I'm wondering about this: With CJS support dropped, why does the .prettierrc.cjs file still need to be in CJS format?

Shinigami92 commented 1 year ago

I'm wondering about this: With CJS support dropped, why does the .prettierrc.cjs file still need to be in CJS format?

Uhm... you could be right, it doesn't need to be cjs anymore

lehni commented 1 year ago

I just tested with prettier.config.js, and it works. Looks nicer too:

// @ts-check
/// <reference types="@prettier/plugin-pug/src/prettier" />

/**
 * @type {import('prettier').Options}
 */
export default {
  plugins: ['@prettier/plugin-pug'],
  …
}