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

v3.0.0 #458

Merged
merged 1 commit into from Jul 26, 2023
Merged

v3.0.0 #458

merged 1 commit into from Jul 26, 2023

Conversation

Shinigami92
Copy link
Member

@Shinigami92 Shinigami92 commented Jul 24, 2023

closes #411
closes #456

@Shinigami92
Copy link
Member Author

Shinigami92 commented Jul 24, 2023

@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 Shinigami92 requested review from fisker and lehni July 24, 2023 15:44
@Shinigami92 Shinigami92 marked this pull request as ready for review July 24, 2023 15:44
@Shinigami92
Copy link
Member Author

And here are the rendered changelogs

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

@lehni
Copy link
Collaborator

lehni commented Jul 24, 2023

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

@fisker
Copy link
Member

fisker commented Jul 25, 2023

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

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

package.json Outdated
"exports": {
".": {
"types": "./dist/index.d.ts",
"import": "./dist/index.js",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line can be removed.

@Shinigami92
Copy link
Member Author

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

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

fisker commented Jul 25, 2023

Is this info not sufficient enough?

Missed that.

fisker
fisker previously approved these changes Jul 25, 2023
@lehni
Copy link
Collaborator

lehni commented Jul 25, 2023

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

lehni commented Jul 25, 2023

@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
Copy link
Collaborator

lehni commented Jul 25, 2023

This might be our solution too: vitejs/vite#11552 (comment)

@Shinigami92
Copy link
Member Author

@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
Copy link
Collaborator

lehni commented Jul 25, 2023

@Shinigami92 I got it running like this:

lehni@3b2ce18

Now trying your version

@lehni
Copy link
Collaborator

lehni commented Jul 25, 2023

@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
lehni previously approved these changes Jul 25, 2023
@lehni
Copy link
Collaborator

lehni commented Jul 25, 2023

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

@Shinigami92
Copy link
Member Author

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

lehni commented Jul 25, 2023

Then merge away, I'd say 🎉

@Shinigami92
Copy link
Member Author

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

lehni commented Jul 25, 2023

@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
Copy link
Collaborator

lehni commented Jul 25, 2023

@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

Co-authored-by: fisker <lionkay@gmail.com>
Co-authored-by: Jürg Lehni <juerg@scratchdisk.com>
@Shinigami92
Copy link
Member Author

@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
Copy link
Collaborator

lehni commented Jul 26, 2023

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

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 🤔

@lehni
Copy link
Collaborator

lehni commented Jul 26, 2023

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

@Shinigami92
Copy link
Member Author

@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 🤔
Did you pull the latest change?

@Shinigami92
Copy link
Member Author

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

@Shinigami92 Shinigami92 merged commit a206d9c into main Jul 26, 2023
11 checks passed
@Shinigami92 Shinigami92 deleted the v3 branch July 26, 2023 16:23
@lehni
Copy link
Collaborator

lehni commented Jul 26, 2023

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

lehni commented Jul 26, 2023

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

@lehni
Copy link
Collaborator

lehni commented Jul 26, 2023

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

@Shinigami92
Copy link
Member Author

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

lehni commented Jul 26, 2023

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'],}

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.

None yet

3 participants