Migrate prettier/prettier to prettier/pre-commit

[joao-p-marques]

Prettier installation is currently broken. See prettier/prettier#9459

Steps to reproduce

Locally

If you have prettier cached, clean with pre-commit uninstall, and run pre-commit again. You will get an error like:

return code: 1
expected return code: 0
stdout: (none)
stderr:
    npm ERR! code ERESOLVE
    npm ERR! ERESOLVE unable to resolve dependency tree
    npm ERR! 
    npm ERR! While resolving: prettier@1.19.1
    npm ERR! Found: @angular/compiler@8.2.13
    npm ERR! node_modules/@angular/compiler
    npm ERR!   @angular/compiler@"8.2.13" from the root project
    npm ERR! 
    npm ERR! Could not resolve dependency:
    npm ERR! peer @angular/compiler@"^6.0.0 || ^7.0.0" from angular-estree-parser@1.1.5
    npm ERR! node_modules/angular-estree-parser
    npm ERR!   angular-estree-parser@"1.1.5" from the root project
    npm ERR! 
    npm ERR! Fix the upstream dependency conflict, or retry
    npm ERR! this command with --force, or --legacy-peer-deps
    npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
    npm ERR! 

On a CI

If you have a CI system that dynamically pulls and installs prettier in each run, it will happen every time.

Solution

Migrating from https://github.com/prettier/prettier/ to https://github.com/prettier/pre-commit/ (a mirror specifically for pre-commit) solves the problem.

Even if this is a temporary problem, there are plans to fully migrate to prettier/pre-commit in the future (prettier/prettier#8937)

We also need to add extra args and additional_dependencies from upstream prettier in this case (prettier/pre-commit#16 (comment)) and add the "v" in the ref because the repo structure is different.

ping @pedrobaeza @sbidoul @yajo

[pedrobaeza]

@sbidoul is there a tool for propagating this change to all repos (13.0 and 14.0)? Is this the problem mentioned in other threads?

[sbidoul]

is there a tool for propagating this change to all repos

For 14.0 yes, based on copier update. I'll finalize it and push to maintainer-tools it when I push v1.0.3.

For 13.0 it's more difficult.

I see @yajo is involved in the linked prettier issues. Let's wait to see what comes out of it.

@lmignon had a similar issue on one of our private repos this morning and he could resolve it by pinning the node version like we do on the 14.0 branches. That might be enough as a workaround?

[joao-p-marques]

Thanks for the response.

Indeed, seems like this issue can be fixed by pinning the npm version: prettier/prettier#9459 (comment) and pre-commit/pre-commit#1655

However, as I mentioned above, there seems to be plans to completely migrate prettier/prettier to prettier/pre-commit. In that case, it would be necessary to make this change anyway.

WDYT @yajo?

[pedrobaeza]

Pinning the npm version means also to change all OCA repos?

[joao-p-marques]

I just noticed #17
According to pre-commit/pre-commit#1655 (comment), that should fix the issue. I have also tested locally and it seems to work.
As @pedrobaeza was suggesting, how could that be propagated to the repos? Below v13, would that have to be a manual action in each repo?

[pedrobaeza]

But if it needs the same propagation, I prefer the definitive patch.

[joao-p-marques]

Yes, that would be better, indeed.

[yajo]

The problem here is that we don't know what will be the outcome of prettier/pre-commit#18 and prettier/pre-commit#17, so maybe it's better to stick to just changing language version and let these things evolve a bit before we do this change.

[yajo]

Below v13

Below v13 there's no pre-commit, so it doesn't affect. It's just v13.

I just noticed #17

Indeed that's the definitive minimal fix. I just tested locally.

[joao-p-marques]

Ok, then what would be the best way to propagate it?

[yajo]

Probably a handcrafted script.

[yajo]

Given what I said above in #33 (comment) and that #17 was already merged and prevents prettier/prettier#9459 from happening, my vote goes for closing this PR. When we prepare the new templates for v15, we'll see if prettier/pre-commit#18 and prettier/pre-commit#17 are fixed and what to do about it.

Do you agree? Thanks for the patch anyway 😊

[joao-p-marques]

Well, fine by me 😄
I just wanted a way to fix this... What would be the path to take to that script you mentioned? Would it be something for the OCA Bot?

[yajo]

@sbidoul knows better...

[sbidoul]

@yajo what needs to be done exactly? Pin the node version in the 13.0 branches?

[pedrobaeza]

That change can be done with OCA/maintainer-tools#442

[yajo]

@yajo what needs to be done exactly? Pin the node version in the 13.0 branches?

Yes, that's it.

[sbidoul]

Pin the node version in the 13.0 branches?

This is now applied on all 13.0 branches.

[lmignon]

Thank you @sbidoul !