Repository navigation
Add prettier, eslint and stylelint to the root package.json - #84017
Conversation
🤖 PR meta 🤖🎉 PropsIf you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. Updated as activity occurs, without notifying anyone named here. Add the 📦 Bundle sizeSize Change: 0 B Total Size: 8.3 MB
⚡ PerformanceShow the resultsClient side metrics exclude the server response time. front-end-block-theme
front-end-classic-theme
media-processing
media-upload
post-editor
site-editor
🏁 Flaky testsSome tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information. should update the URL from the last navigation if only varies in the URL fragment in
|
|
The PR updated the docs to show the required changes needed for VS Code settings |
|
So the settings you need in your gitignored VS Code settings is this // Use TypeScript from the workspace instead of VS Code's built-in TypeScript version.
"js/ts.tsdk.path": "tools/build-scripts/node_modules/@typescript/native/lib",
// Use ESLint, Prettier and Stylelint from the respective paths.
"eslint.nodePath": "tools/eslint/node_modules",
"prettier.prettierPath": "packages/prettier-config/node_modules/prettier",
"stylelint.stylelintPath": "tools/stylelint/node_modules/stylelint", |
|
To be clear, I don't think we should add prettier to root package.json, the contributors instead need to update the VS Code settings as mentioned above. |
|
Oh, I see. So we should close this PR, or at most tweak the docs to suggest the correct IDE settings? |
Correct. |
|
We had the same conversation some time ago when discussing the TypeScript plugin issue. Why would we force everyone to do a custom bit of configuration when we can just have the tools and their config files in the default location where all plugins reasonably expect it? It's a degradation of developer experience without getting anything in return. |
I would argue that we should maintain that VS Code configuration which was blocked in #77913. There are downsides to adding dependencies to root, which were cleaned up via #75041. |
|
IMHO, we have a good dedicated docs section about VS Code, which should suffice the integration |
|
@manzoorwanijk @ciampo How do you deal with the argument that repos should be zero-config for IDEs if possible? Open basically any project on GitHub, and the IDE knows instantly how to format, lint and typecheck files. All things that are done directly in the edited file view, with inline highlights... When regularly working with 5+ repos and often checking out new ones, you don't need to worry whether a particular repo has TypeScript in I'm quite suprised that we're ready to give up and dismiss such a self-evident benefit with such an ease. Now I found out that the inline linting is now also broken, because there is no longer any The |
That is exactly what #77913 initially tried to do - to have a better integration (not just prettier) as soon as you open the IDE. |
|
As an exception, we can add a few deps like ESLint, Prettier, and TypeScript to the root just to satisfy the IDEs, but since we have packages that have those as dependencies which means they will end up using the root dependencies if their own copies get removed, affecting their consumers. If we are fine, I would love to bring back the initial version of #77913, which uses the recommended settings for VS Code. |
|
One strong argument for having that managed custom config is the |
OK, I remember that now, it wasn't just a docs update, but it provided the default config, too 🙂 That would satisfy my request to make it zero-config from the user perspective. I'm for revisiting that PR. The arguments against were not very strong, and mostly cultural. WordPress tries to be IDE agnostic, a similar patch for
That looks like a good tradeoff to me, getting IDE zero-config by default, versus missing dependencies not being immediately detected. |
Yes, I will create a PR to bring it back.
That still doesn't solve the |
|
I have created #84025 to bring the managed VS Code settings back. |
|
I ran into a few issues today that seem to track back to recent changes for isolated workspaces, even after #84025:
Flagging these largely as points of contributor friction, in support of the concerns raised by @jsnajdr . Even now, with all of our documentation, I don't know what I'm supposed to do. "Install VS Code Settings" doesn't show up as a command. I assume I probably need to do something about removing and merging my own custom settings. It's all very unclear. I'm not sure I follow what the problems are with this pull request, as-is? Or maybe there's some other option, like simulating how IDEs will look for |
That might be related to #84057 (comment) |
Editor extensions look for these tools in the root node_modules, the same as prettier. The versions match the workspaces that use them, so npm links the same copies.
The extensions now find these tools in the root node_modules, so only the TypeScript 7 path is still needed.
The concern is only about the dependencies in root which make them available to our prettier and eslint packages, but I think having these 3 deps in root is a tradeoff which has benefits. I have updated the PR to add the required deps to root. |
manzoorwanijk
changed the title
manzoorwanijk
left a comment
manzoorwanijk
left a comment
There was a problem hiding this comment.
Let us go ahead with this PR. Someone with privileged access will need to merge it without that failing required check which fails because of the newly added deps in root.
|
I just merged this, bypassing the required rules, namely the root dependencies check. |
|
Thanks all, I think this is a good pragmatic outcome to reduce friction, at least in the short-term 👍 |
4 participants
What?
Follow up to #83847 and #84025. Adds
prettier,eslintandstylelintto the rootpackage.json.Why?
Fixing a regression introduced by the isolated deps in #83847. The Prettier, ESLint and Stylelint plugins in Cursor, VS Code or any other IDE no longer find the versions the repo installs (for Prettier, the
wp-prettierfork) and use their default built-in ones. That's because these dependencies are not declared in the rootpackage.json, so they are no longer present in the rootnode_modules: nothing hoists them there from subfolders anymore.The result is, for example, that the "Format Document" command in VS Code doesn't apply the
parenSpacingoption.The managed VS Code settings from #84025 work around this with custom paths, but only in VS Code and only for contributors who use the managed settings file (see #84017 (comment)).
How?
.vscode/settings.dist.jsonc; only the TypeScript 7 settings remain.Testing Instructions
npm installparenSpacingis applied.Testing Instructions for Keyboard
N/A
Screenshots or screencast
N/A