-
-
Notifications
You must be signed in to change notification settings - Fork 44
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
Update prettier to v3 #411
Conversation
@fisker I need docs for how to migrate a plugin |
|
actually the 2nd and the 3rd are actually all options now
@fisker Did the |
parsers all async. (All most, but all should be |
I didn't see you call parse. What's wrong? |
Ah I assume it has something todo with: https://github.com/prettier/prettier/blob/1a602b590ec9ed5db01780939659c427e3d277b8/changelog_unreleased/api/13268.md |
There are 3 failing tests: ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Failed Tests 3 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
FAIL tests/syntax-errors/syntax-errors.test.ts > Syntax-Errors > should format non-JS script without syntax error
Error: promise resolved "// Copied from JSON-LD playground
script(type="application/ld+json").
{
"@context": {
"name": "http://schema.org/name",
"description": "http://schema.org/description",
"image": {
"@id": "http://schema.org/image",
"@type": "@id"
},
"geo": "http://schema.org/geo",
"latitude": {
"@id": "http://schema.org/latitude",
"@type": "xsd:float"
},
"longitude": {
"@id": "http://schema.org/longitude",
"@type": "xsd:float"
},
"xsd": "http://www.w3.org/2001/XMLSchema#"
},
"name": "The Empire State Building",
"description": "The Empire State Building is a 102-story landmark in New York City.",
"image": "http://www.civil.usherbrooke.ca/cours/gci215a/empire-state-building.jpg",
"geo": {
"latitude": "40.75",
"longitude": "73.98"
}
}
// Copied from https://YAML.org
script(type="application/x-not-a-real+yaml").
%YAML 1.2
---
YAML: YAML Ain't Markup Language
What It Is: YAML is a human friendly data serialization
standard for all programming languages.
YAML Resources:
YAML 1.2 (3rd Edition): http://yaml.org/spec/1.2/spec.html
YAML 1.1 (2nd Edition): http://yaml.org/spec/1.1/
YAML 1.0 (1st Edition): http://yaml.org/spec/1.0/
YAML Issues Page: https://github.com/yaml/yaml/issues
YAML Mailing List: yaml-core@lists.sourceforge.net
YAML IRC Channel: "#yaml on irc.libera.chat"
YAML Reference Parser: http://ben-kiki.org/ypaste/
YAML Spec: https://github.com/yaml/yaml-spec
YAML Test Suite: https://github.com/yaml/yaml-test-suite
YAML Test Matrix: https://matrix.yaml.io/
YAML Docker Runtimes: https://github.com/yaml/yaml-runtimes
YAML Cookbook (Ruby): YAML_for_ruby.html
Projects:
C/C++:
- libfyaml # "C" YAML 1.2 processor | YTS
- libyaml # "C" Fast YAML 1.1 | YTS
- libcyaml # YAML de/serialization of C data structures (using libyaml)
- yaml-cpp # C++ YAML 1.2 implementation
Crystal:
- YAML # YAML 1.1 from the standard library
C#/.NET:
- YamlDotNet # YAML 1.1/(1.2) library with serialization support | YTS
- yaml-net # YAML 1.1 library
D:
- D-YAML # YAML 1.1 de/serialization library with official community support | YTS
Dart:
- yaml # YAML package for Dart
Delphi:
- Neslib.Yaml # YAML 1.1 Delphi binding to libyaml | YTS
Golang:
- Go-yaml # YAML support for the Go language.
- Go-gypsy # Simplified YAML parser written in Go.
- goccy/go-yaml # YAML 1.2 implementation in pure Go.
Haskell:
- HsYAML # YAML 1.2 implementation in pure Haskell | YTS
- YamlReference # Haskell 1.2 reference parser
- yaml # YAML 1.1 parser/renderer for Haskell ( ❯ async /Users/shinigami/OpenSource/Shinigami92/plugin-pug/tests/syntax-errors/syntax-errors.test.ts:35:5
based on libyaml)
Java:
- SnakeYAML # Java 5 / YAML 1.1
- YamlBeans # To/from JavaBeans. YAML 1.0/1.1
- eo-yaml # YAML 1.2 for Java 8. Also packaged as a Module (Java 9+).
JavaScript:
- js-yaml # Native PyYAML port to JavaScript. Demo
- yaml # JavaScript parser and stringifier (YAML 1.2, 1.1) | YTS
Nim:
- NimYAML # YAML 1.2 implementation in pure Nim | YTS
OCaml:
- ocaml-syck # YAML 1.0 via syck bindings
Perl Modules:
- YAML # Pure Perl YAML 1.0 Module
- YAML::XS # Binding to libyaml
- YAML::Syck # Binding to libsyck
- YAML::Tiny # A small YAML subset module
- YAML::PP # A YAML 1.2/1.1 processor | YTS
PHP:
- The Yaml Component # Symfony Yaml Component - Loads and dumps YAML files (YAML 1.2)
- php-yaml # libyaml bindings (YAML 1.1)
- syck # syck bindings (YAML 1.0)
- spyc # yaml loader/dumper (YAML 1.?)
Python:
- PyYAML # YAML 1.1, pure python and libyaml binding
- ruamel.yaml # YAML 1.2, update of PyYAML with round-tripping of comments
- PySyck # YAML 1.0, syck binding
- strictyaml # Restricted YAML subset
R:
- R YAML # libyaml wrapper
Ruby:
- psych # libyaml wrapper (in Ruby core for 1.9.2)
- RbYaml # YAML 1.1 (PyYAML Port)
- yaml4r # YAML 1.0, standard library syck binding
Rust:
- yaml-rust # YAML 1.2 implementation in pure Rust
- serde-yaml # YAML de/serialization of structs
Others:
- yamlvim (src) # YAML dumper/emitter in pure vimscript
script(type="text/typescript").
const myVariable: string = "Hello World" as any as string;
function myFunc<myGeneric>({ myProp = 42 }: { myProp: number }): myGeneric {
console.log(myVariable);
re
turn true;
}
myFunc({ myProp: 100 });
script(type="text/markdown").
# h1
## h2
### h3
#### h4
##### h5
###### h6
Hello World.
- Bullet point 1
- Bullet point 2
1. List item
2. Another list item
" instead of rejecting
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/3]⎯ FAIL tests/issues/issue-70/issue-70.test.ts > Issues > should handle angular framework interpolation
AssertionError: expected 'div\n | {{ "foo" | baz : bar }}\n\n.…' to be 'div\n | {{ "foo" | baz: bar }}\n\n.h…' // Object.is equality
❯ tests/issues/issue-70/issue-70.test.ts:9:19
7| formatOptions: { pugFramework: 'angular' },
8| });
9| expect(actual).toBe(expected);
| ^
10| });
11| });
- Expected - 1
+ Received + 1
div
- | {{ "foo" | baz: bar }}
+ | {{ "foo" | baz : bar }}
.headline {{ $t("langkey", { id: `${model.id}` }) }}
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[2/3]⎯ FAIL tests/issues/issue-67/issue-67.test.ts > Issues > should handle Angular expressions
❯ tests/issues/issue-67/issue-67.test.ts:7:19
AssertionError: expected '.foo([myAttribute]="{ param: \'hello …' to be '.foo([myAttribute]="{ param: \'hello …' // Object.is equality
5| it('should handle Angular expressions', async () => {
6| const { expected, actual } = await compareFiles(__dirname);
7| expect(actual).toBe(expected);
| ^
8| });
9| });
- Expected - 1
+ Received + 1
- .foo([myAttribute]="{ param: 'hello {name}' | translate: { name: arg } }")
+ ".foo([myAttribute]="{ param: 'hello {name}' | translate : { name: arg } }")
.foo(*ngIf="myObjectAsync | async as myObject; else fallbackTemplate")
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[3/3]⎯ Test Files 3 failed | 125 passed (128)
Tests 3 failed | 297 passed (300)
Start at 11:12:06
Duration 10.68s (setup 5ms, collect 11.25s, tests 8.85s)
FAIL Tests failed. Watching for file changes...
press h to show help, press q to quit |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, wrong comment. |
Ah, I forgot it's expected prettier/prettier#13100 |
Anyway, do you aware this https://github.com/prettier/prettier/blob/f2244bdc8621c0a001e3717f595bc8fd8653cce7/src/language-html/embed.js#L157? Not sure how we handle here. |
Tests are running green locally 🥳 |
Great! |
# Conflicts: # package.json # pnpm-lock.yaml # src/printer.ts
prettier now ships its own types! prettier/prettier#14033 |
What would be required to ship a prerelease on npm ( |
a manual interaction by me |
@carmanchris31 I will first update the project to TS 5 in next few days |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fisker this is a LOT of await
calls
Why is this needed that much for a relative linear top to bottom IO file write?
// @ts-expect-error -- Prettier allow it to be async if we don't do recursively print | ||
async print( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prettier allow it to be async if we don't do recursively print
So why is there no overloading like:
print(
path: AstPath<T>,
options: ParserOptions<T>
): Doc;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't understand what do mean.
@@ -131,8 +117,7 @@ export const plugin: Plugin = { | |||
/** The languages that are picked up by prettier. */ | |||
export const languages: SupportLanguage[] | undefined = plugin.languages; | |||
/** The parsers object that is picked up by prettier. */ | |||
export const parsers: { [parserName: string]: Parser } | undefined = | |||
plugin.parsers; | |||
export const parsers = plugin.parsers; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fisker I would prefer using explicit types, so it runs into a compile-time error when something changes
export const parsers = plugin.parsers; | |
export const parsers: | |
| { [parserName: string]: Parser | (() => Promise<Parser>) } | |
| undefined = plugin.parsers; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed it because I don't know how to define types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Who is the most TS experienced Prettier core maintainer in your Team?
But you should also start to learn TS 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't know, because we don't use TS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But you now have the types inside Prettier 🙃
So the team is responsible for the types 👀
But I can try to contribute the TS type overload for print
and see if it is possible to also add a non async version for it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's already defined as sync function, and we decide to keep the async support undocumented. prettier/prettier#13211 (comment)
@@ -2,7 +2,7 @@ import type { ChoiceSupportOption, PathArraySupportOption } from 'prettier'; | |||
import { CATEGORY_PUG } from '../constants'; | |||
|
|||
const pugSortAttributesOption: PathArraySupportOption = { | |||
since: '1.7.0', | |||
// since: '1.7.0', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fisker What happend to the since
property?
Why will it be remove in v3? Can you provide some background info?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it work before? If it works, since "what"? Version of Prettier? Version of this plugin(Prettier doesn't know version of plugin)?
Even for builtin plugins. We can't find how to use it.
That's why its removed. Make sense to you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously in @types/prettier
it was a required property (see https://github.com/DefinitelyTyped/DefinitelyTyped/blob/d8c6e21d99e00769b450c6e091a85eb2eb4e63b3/types/prettier/index.d.ts#L554)
I always thought this was for documentation purposes 🤔
But if you/Prettier removed the support for it, then I can also remove it instead of just commenting them out and produce theoretically dead code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I comment it out to let you decide what to do.
I didn't get which part are you talking about. |
In every test we now need to call |
I still can't understand the problem here. Are you saying |
@@ -467,14 +473,14 @@ export class PugPrinter { | |||
options.semi = false; | |||
} | |||
|
|||
let result: string = format(code, options); | |||
let result: string = await format(code, options); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see, this format
is async
😕
@Shinigami92 what's blocking this now? Can I be of help? |
Before I upgrade plugin-pug, I want to have a released working stable eslint-config-prettier, eslint-plugin-prettier and prettier-plugin-organize-imports Then I need to find also the time and will, but right now company work is exhausting |
eslint-config-prettier, eslint-plugin-prettier and prettier-plugin-organize-imports have prettier 3.0 compatible releases btw. |
@Shinigami92 I'm happy to put some time into wrapping up this PR. Perhaps you could outline what remains to be done? |
I'm on mobile right now, it is more like I don't have the time to look what is missing I can't promise anything but maybe I can invest some time tomorrow |
@Shinigami92 I've rebased the branch on main and adjusted the prettier versions here: I get only three failing tests, all related to the same topic of angular expressions. I am not sure what adds the extra space there…. @fisker perhaps you'd have some insight?
|
So these somehow relate to 6fd56df but I'm not sure what's the correct behavior |
With lehni@33089ab, all the tests past in my rebased branch. |
I needed another fix to be able to build the TS files, and with this, the plugin works perfectly in my code-base (tested with |
I commit 6fd56df because prettier/prettier#13100 Later, we changed it again prettier/prettier#14961 |
You can send a PR to |
I've rebased again and fixed the remaining linting issues. All tests are green now: #456 |
Prettier v3 is a fully breaking change