zenstackhq / zenstack

Fullstack TypeScript toolkit that enhances Prisma ORM with flexible Authorization layer for RBAC/ABAC/PBAC/ReBAC, offering auto-generated type-safe APIs and frontend hooks.
https://zenstack.dev
MIT License
2.07k stars 88 forks source link

Update Prettier `^2.8.0` -> `>= 2.8, <4.0` #612

Closed sitch closed 10 months ago

sitch commented 1 year ago

Bump prettier and @types/prettier to support the new 3.0 release

ymc9 commented 1 year ago

Hi @sitch , is this about version of Prisma or Pritter?

sitch commented 1 year ago

Ah sorry I meant Prettier!

ymc9 commented 1 year ago

Ah sorry I meant Prettier!

Got it 😄

sitch commented 11 months ago

All this needs is to add an await here: https://github.com/zenstackhq/zenstack/blob/6fb649ea7a9d43d0f74fc2db7b1f089d7bc6afee/packages/sdk/src/code-gen.ts#L19

const formatted = await prettier.format(content, formatOptions);

to match the new async interface: https://github.com/prettier/prettier/blob/main/src/index.d.ts#L563 and to bump some dependency ranges

ymc9 commented 11 months ago

All this needs is to add an await here:

https://github.com/zenstackhq/zenstack/blob/6fb649ea7a9d43d0f74fc2db7b1f089d7bc6afee/packages/sdk/src/code-gen.ts#L19

const formatted = await prettier.format(content, formatOptions);

to match the new async interface: https://github.com/prettier/prettier/blob/main/src/index.d.ts#L563 and to bump some dependency ranges

Got it @sitch . Wil make a fix in the next release!

sitch commented 10 months ago

Need a PR for this? It should just need the await mentioned above and changes to the package.json dependencies. Currently, if prettier 3.0 is used, zenstack won't write any files due to this empty try/catch https://github.com/zenstackhq/zenstack/blob/6fb649ea7a9d43d0f74fc2db7b1f089d7bc6afee/packages/sdk/src/code-gen.ts#L23

An alternative solution would be to untangle the current try/catch to cover sourceFile saving and sourceFile formatting separately. This would allow zenstack to continue working (albeit without formatting) with a prettier 3.0 project resolution

ymc9 commented 10 months ago

Hi @sitch , my apologies that this fell through the cracks. The "SDK" package has a direct dependency on prettier "^2.8.3", so I'm surprised why the version that the user installed breaks it ... maybe I missed something here. Was the interface actually changed in newest 2.x release? Upgrading to 3.x sounds good to me. I'm just curious about why it broke ...

But I agree with you we should make sure the files are always written. A PR is greatly appreciated if you have time for it. Thank you!

sitch commented 10 months ago

The interface from v2 -> v3 changes the format call to return a Promise<string> instead of string. So just adding an await beforehand and setting the version to 2.x || 3.x would be all that's required.

So having multiple versions of prettier breaks things in my current monorepo, so things break if I force a package.json resolution for prettier v3. And because prettier.format now returns a promise, this function: https://github.com/zenstackhq/zenstack/blob/6fb649ea7a9d43d0f74fc2db7b1f089d7bc6afee/packages/sdk/src/code-gen.ts#L19 now throws an error, and as a result, this line https://github.com/zenstackhq/zenstack/blob/6fb649ea7a9d43d0f74fc2db7b1f089d7bc6afee/packages/sdk/src/code-gen.ts#L21 is not called, so no files get saved.

As for the PR, do you want the await and package.json bump one or, one that just splits the try/catch into two steps: 1) code formatting, and 2) code saving?

ymc9 commented 10 months ago

The interface from v2 -> v3 changes the format call to return a Promise<string> instead of string. So just adding an await beforehand and setting the version to 2.x || 3.x would be all that's required.

So having multiple versions of prettier breaks things in my current monorepo, so things break if I force a package.json resolution for prettier v3. And because prettier.format now returns a promise, this function:

https://github.com/zenstackhq/zenstack/blob/6fb649ea7a9d43d0f74fc2db7b1f089d7bc6afee/packages/sdk/src/code-gen.ts#L19

now throws an error, and as a result, this line https://github.com/zenstackhq/zenstack/blob/6fb649ea7a9d43d0f74fc2db7b1f089d7bc6afee/packages/sdk/src/code-gen.ts#L21

is not called, so no files get saved. As for the PR, do you want the await and package.json bump one or, one that just splits the try/catch into two steps: 1) code formatting, and 2) code saving?

Thanks for the clarification @sitch .

I think it should be good to just bump version and add the await there. The reason why it was broken should be that the source file's content is replaced with a Promise ... We don't really need to do per-file saving, because the source files will be saved when project.save() is called.

ymc9 commented 10 months ago

Fixed by #888