Do not declare wp-prettier as a dependency of @wordpress/eslint-plugin

[nicholasio]

What problem does this address?

Currently, the @wordpress/eslint-plugin is declaring the WordPress fork of prettier as direct dependency and therefore making the wrong assumption that every project using that package is going to use WP fork of prettier.

At 10up, we have our own build tool similar to wp-scripts but we do not use wp-prettier, we use prettier original implementation. Our eslint config exports a wordpress config that makes use of the custom @wordpress eslint rules but depending on @wordpress/eslint-plugin causes some issues (from npm ERESOLVE warnings to errors in some versions of npm) due to conflicting dependencies of prettier.

IMO prettier and wp-prettier have nothing to do with @wordpress/eslint-plugin and it should be up to the consumers of the plugin to decide how they want to format their code.

What is your proposed solution?

Remove wp-prettier from package.json in the eslint-plugin package

gutenberg/packages/eslint-plugin/package.json

Line 49 in ab65bf4

"prettier": "npm:[email protected]",

and have wp-scripts declare prettier's fork as a direct dependency.

[gziolo]

In theory, we could make Prettier a peer dependency and mark it as optional. It would be similar to what we did for TypeScript:

gutenberg/packages/eslint-plugin/package.json

Lines 57 to 61 in 6933f93

	"peerDependenciesMeta": {
	"typescript": {
	"optional": true
	}
	},

gutenberg/packages/eslint-plugin/configs/recommended.js

Line 30 in 6933f93

if ( isPackageInstalled( 'typescript' ) ) {

We need to evaluate what implications that would have. Many projects use only @wordpress/eslint-plugin so they probably don't install explicitly prettier in any form. In the case when someone using it indirectly with @wordpress/scripts it would probably work exactly the same as today.

In general, working with this Prettier's fork has been painful for quite some time as reported in #21872. I shared a proposal so we stop using the fork in WordPress/Gutenberg some time ago: https://make.wordpress.org/core/2022/01/05/proposal-changes-to-javascript-coding-standards-for-full-prettier-compatibility/.

I updated my test repository today that uses @wordpress/scripts and I had to explicitly install this fork when using npm 8 so it doesn't use the regular Prettier:

https://github.com/gziolo/wp-block-directory-template/blob/2ed0d0f0e1a34a18850f0ef794b9714df5a9dd4e/package.json#L27

I saw some warnings because of that ...

IMO prettier and wp-prettier have nothing to do with @wordpress/eslint-plugin and it should be up to the consumers of the plugin to decide how they want to format their code.

Actually, I have to disagree here for the context of WordPress. We use Prettier + ESLint integration to enforce WordPress JavaScript conventions. Anyway, I agree that we can make it more flexible as noted above.

[nicholasio]

I thought about the peerDep and making it optional. I just wasn't sure how NPM would handle an optional dep with name of prettier and a different version when other packages also require the same dep name but with a different version.

I believe the optional dep idea would be easier if we could tell eslint to use a different prettier binary (unsure if this is possible) .That way you could declare two optional peer deps: wp-prettier (which would expose a wp-prettier bin) and prettier.

[ocean90]

I updated my test repository today that uses @wordpress/scripts and I had to explicitly install this fork when using npm 8 so it doesn't use the regular Prettier:

Just confirming that npm 7/8 currently do not resolve aliases defined in dependencies. I have also started to add the alias to the main package.json. Gutenberg itself has also defined it for a long time already.

Related: npm/cli#4197, npm/cli#2800

[gziolo]

Gutenberg itself has also defined it for a long time already.

It worked way much better with npm 6, but sometimes it was problematic in Gutenberg so we enforced it to avoid issues. In my testing with npm 6 these npm aliases worked quite well but we had reports about issues.

[nicholasio]

as FYI this is what npm ls look like for prettier when using npm 8 on 10up-toolkit. I've had reports where npm install completely fails when using npm 7. npm 8 just gives an ERESOLVE warning.

If consumers of @wordpress/eslint-plugin are already having issues with the alias and are having to add the alias manually I'm guessing we could just remove the alias from @wordpress/eslint-plugin?

[gziolo]

If consumers of @wordpress/eslint-plugin are already having issues with the alias and are having to add the alias manually I'm guessing we could just remove the alias from @wordpress/eslint-plugin?

I think we should just mark a regular prettier as peerDependency that is optional. The remaining question is whether it is still going to be installed with npm 8 because it's marked as a peer dependency for eslint-plugin-prettier. Maybe we would have to mark eslint-plugin-prettier as an optional peer dependency as well.

[nicholasio]

If consumers of @wordpress/eslint-plugin are already having issues with the alias and are having to add the alias manually I'm guessing we could just remove the alias from @wordpress/eslint-plugin?

I think we should just mark a regular prettier as peerDependency that is optional. The remaining question is whether it is still going to be installed with npm 8 because it's marked as a peer dependency for eslint-plugin-prettier. Maybe we would have to mark eslint-plugin-prettier as an optional peer dependency as well.

That makes sense to me re: including eslint-plugin-prettier as optional peer dependency.

[gziolo]

I opened #39244 to make prettier integration optional in the ESLint plugin. I'm still not sure whether eslint-plugin-prettier needs to be moved to peer dependencies as well, but we will find out soon.

[gziolo]

@nicholasio, there is @wordpress/[email protected] available with the fix. Can you check if that resolves the issue?

[nicholasio]

@gziolo Thanks!

This solved the issue for me

[gziolo]

@nicholasio, this is great news and thank you for reporting back 🎉