Compatibility with Yarn 2 (berry)

[kachkaev]

I was playing with Yarn 2 and discovered that the linting tools required an additional setup in order to be integrated with VSCode. After running yarn set version berry; yarn install and updating .gitignore, I also had to run this somewhat unusual command:

yarn dlx @yarnpkg/pnpify --sdk vscode   

It created a bunch of files in yarn/sdks and also added .vscode/settings.json with the links to the new entry points for the tools. That affected ESLint, Prettier and TypeScript, but not Markdownlint.

I'm opening this issue to explore what’s needed for a local Makrdownlint to become compatible with VSCode when a project is configured with Yarn 2. The adoption of its new ‘Plug and Play’ architecture is still low, but I guess that the raise is inevitable.

As of now, it seems that VSCode falls back to the global version of Markdownlint, which may be different from the one mentioned in devDependencies. Besides, .markdownlint.json is ignored if the file extends a config in a local npm package. The latter will be fixed for CLI via DavidAnson/markdownlint#342, but we'll probably need to do something extra to fix the SDK integration as well.

I won’t have time this weekend, but happy to join the investigation and also help with the testing in the upcoming weeks.

[DavidAnson]

The VS Code extension does not depend on any version of the library being installed. It bundles its own version and uses that. So I am guessing there doesn't need to be any path redirection as with those other tools.

The extends implementation used by the VS Code extension is the same one that is exported by the library, so it should automatically benefit from your changes there once the dependency is updated.

Therefore, it doesn't seem like there should be any special work needed here. But please let me know if you find otherwise.

[kachkaev]

I see what you mean. I have not noticed that the extension does not resolve a local version of markdownlint and always uses a built-in one. This behaviour of other linters made me think Markdownlint is doing something similar.

Perhaps, it could be worth adding something like markdownlint.markdownlintPath, which would allow project to specify some kind of an entry point command. Otherwise, "extends": "@my-company/markdownlint-config" inside a config file will not work in Yarn 2 Berry or Yarn 1 + PnP. The reason for other tools to provide the entrypoint is to change how module.resolve works and hence let the tools correctly discover the configs and plugins.

[DavidAnson]

I'd love to let the other tools (with actual teams behind them) figure this out. Per this section, it seems extensible configuration in ESLint still has issues with Yarn's PNP: https://yarnpkg.com/features/pnp#native-support. The idea of Yarn modifying extensions installed via VS Code seems problematic - especially when those extensions get updated automatically (presumably overwriting the customizations).

[kachkaev]

I agree with you that Yarn v2 is still not playing well with the tooling. The support improves over time though and I guess will be good enough quite soon. My experiment with Yarn 2 was somewhat painful this weekend, but I imagine that a year ago the pain would be 10 times more. ESLint seems to be on the path to resolving their compatibility problem and I’m looking forward to trying it out: eslint/eslint#13481!

Let’s try to finish DavidAnson/markdownlint#342 and then come back to this conversation. It’d be great to discuss the missing bits for integrating Markdownlint with VScode and Yarn 2 🙂 Happy to help!

[DavidAnson]

I published new versions of the library and VS Code extension with your changes to handle this scenario better (updated CLI’s to come in a few days). Thanks for the help! Please give this a try and let me know if there’s more to do in this issue.

[kachkaev]

Hi @DavidAnson! Which version of Markdownlint should I try? The latest on npm is 0.21.1, which does not seem to include the fallback: https://www.npmjs.com/package/markdownlint?activeTab=versions. Same for the main repo branch: https://github.com/DavidAnson/markdownlint/commits/main

[DavidAnson]

@kachkaev You're right, I'm sorry for wasting your time! The release I did was the patch release for two minor issues. Your change is in the "next" branch, but not yet part of the release. I got the two mixed up in my head, sorry.

[DavidAnson]

Try now.

[kachkaev]

Just tried the VSCode extension in kachkaev/njt#29 again. Seems like I’m still blocked by lack of DavidAnson/markdownlint#342. If I replace "extends": "@kachkaev/markdownlint-config" with the actual contents of this config, Markdownlint warnings do show up. But I guess that it’s just the built-in markdownlint instance reading my config and thus not involving yarn / package resolution at all.

[DavidAnson]

I'm not sure I understand. Are you saying that you are still blocked because this feature is not yet present in the extension? That makes sense and will be resolved soon, though I thought you were building your own extension to test this? Or are you saying that your scenario doesn't work with the feature as it's been implemented and we should change how it works in the library?

[DavidAnson]

v0.38.0 of this extension is now published with the library changes you made. If that's not enough, please let me know what else you think needs to be done.

[kachkaev]

After upgrading all deps in kachkaev/njt#29, I managed to get Markdownlint working in CI, which is great. However, the VSCode integration is still lacking. Here is what’s show in Markdownlint output:

[1:39:19 PM] INFO: Loading custom configuration from "/path/to/njt/.markdownlint.json", overrides user/workspace/custom configuration for directory and its children.
[1:39:19 PM] ERROR: Unable to read configuration file "/path/to/njt/.markdownlint.json" (ENOENT: no such file or directory, open '/path/to/njt/@kachkaev/markdownlint-config').
[1:39:19 PM] INFO: Loading user/workspace configuration for "/path/to/njt/README.md" (no configuration file in workspace folder).

So the VSCode extension falls back to the default Markdownlint config and incorrectly highlights some bits of the markdown, while ignoring the errors I would like it to report.

My /path/to/njt/.markdownlint.json is:

{
  "extends": "@kachkaev/markdownlint-config",
  "first-line-heading": false,
  "no-inline-html": false
}

That’s similar to, say, my project’s ESLint config, which works:

module.exports = {
  extends: ["@kachkaev/eslint-config-react"],
  rules: {
    // ...
  },
  overrides: [
    // ...
};

The is a significant difference between how Markdownlint VSCode extension works compared to other extensions I’ve been using. TypeScript, ESLint and Prettier extensions try finding a locally installed executable for a given tool and run it in the background. This guarantees the parity between CLI, CI/CD and the editor no matter what. If a tool can resolve a plugin from the command line, so will the VSCode extension, because all it does is just calling that same tool.

As far as I understand, Markdownlint for VSCode runs a built-in version of the library and only looks for the config file in the project. Other linters do this too, but only as a fallback when a local executable is not found or its path is not manually provided. The reason why a built-in library is only fallback option is because it does not guarantee the linting parity between all members of a team. Codebase collaborators can have different editors and different versions of the extension, and if that’s the case, there’ll be discrepancies between contributions or between the feedback people see locally and in CI.

Because the local CLI executable is not involved in the Markdownlint VSCode extension, package resolution fails and so "extends": "@kachkaev/markdownlint-config" cannot be leveraged. Yarn 2 has a built-in package resolution mechanism, so it is not possible to manually search for node_modules/@kachkaev/markdownlint-config or apply other shortcuts potentially available in Yarn 1. That’s why there’s a new concept of Editor SDKs described in the Yarn 2 docs.

To summarise, for package resolution to work in a Yarn 2 project, we need Markdownlint for VSCode to call a local executable, which will be able to load the project’s config and call the right require.resolve function (its behaviour might be non-default). For Yarn 2 PnP there’ll be a need to have an additional editor SDK and a new config param like:

"eslint.nodePath": ".yarn/sdks",
"prettier.prettierPath": ".yarn/sdks/prettier/index.js",

We should probably reopen this issue. Although the prevalence of Yarn 2 is still small, it’s just a matter of time when this issue will be hit by more developers.

[DavidAnson]

I'll need to think about this some more. You are right that this extension uses a bundled version of the library, but this has never been an issue. Teams can easily standardize on extension versions, so I'm not sure it needs to be an issue now, either. I think I understand why the new behavior isn't working as you'd like, but that may not be hard to fix. If you have a project you can point me at that demonstrates the problem, that would be helpful! Otherwise, I'll try to reproduce the scenario and see if it can be addressed by resolving packages relative to the project directory. I'd rather not have to worry about accommodating every different package manager, but I also fear that calling a local executable adds complexity and potential confusion.

[DavidAnson]

Seems like I should be able to use njt to test with? I'll try that first.

[kachkaev]

Thanks for reopening this issue! Indeed, yarn 2 with PnP is bringing quite a few challenges across the ecosystem but the benefits it brings make the efforts to resolve them worthy. What’s good is that many tools are still on the path of supporting PnP, so there is no immediate pressure from production-grade projects on Markdownlint at this point.

If you want to experiment, feel free to use njt’s branch called yarn-berry, which I develop as part of kachkaev/njt#29. Happy to help if I can although I’m still new to PnP myself.

[DavidAnson]

I had a bit of time to look at this. It works today as it should - kind of... What I'm seeing is that if I run the extension in bundled/release mode ("main": "./bundle"), then extends does not work, but if I run it in normal/debug mode ("main": "./extension"), then it DOES work. I suspect the call to require.resolve in resolveConfigExtends in markdownlint.js is behaving differently. This would not be unprecedented as bundling is known to mess with calls to require, etc.. I'll investigate further as time permits.

[DavidAnson]

This seems to be a problem with webpack and is detailed here: webpack/webpack#9393

As in that example, require.resolve needs to do the lookup at run time and return a path. Because that call lives in library code, it can’t be changed to use __non_webpack_require__.

[DavidAnson]

Also relevant: webpack/webpack#4175

webpack does not seem to support the scenario where library code needs to dynamically require something.

[DavidAnson]

I think I have a workaround - though it requires patching markdownlint.

[kachkaev]

Seems like there are still problems with PnP. Using yarn-4.0.0-rc.27. Steps to reproduce:

1. Clone https://github.com/kachkaev/njt
2. cd njt; yarn install (yarn-4.0.0-rc.27 is included in the repo, so you can use globally installed yarn 1)
3. Open the njt folder in the new VScode window
4. Open README.md, start typing
5. Observe this in the output tab:

[9:25:48 AM] ERROR: Exception while linting with markdownlint-cli2:
EntryNotFound (FileSystemError): Unable to read file '/path/to/project/.yarn/__virtual__/@kachkaev-markdownlint-config-virtual-c12d00cf50/6/.yarn/berry/cache/@kachkaev-markdownlint-config-npm-0.5.0-beb10ce28b-9.zip/node_modules/@kachkaev/markdownlint-config/main.json' (Error: Unable to resolve nonexistent file '/path/to/project/.yarn/__virtual__/@kachkaev-markdownlint-config-virtual-c12d00cf50/6/.yarn/berry/cache/@kachkaev-markdownlint-config-npm-0.5.0-beb10ce28b-9.zip/node_modules/@kachkaev/markdownlint-config/main.json')
    at Function._handleError (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:94:29967)
    at Object.readFile (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:94:28881)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

.markdownlintrc.json contains this: { "extends": "@kachkaev/markdownlint-config" }. This is a valid config and it does get picked up by the CLI despite Yarn PnP. This can be confirmed by running yarn lint:markdownlint, then ‘breaking‘ .markdownlintrc.json and running yarn lint:markdownlint again.

[kachkaev]

@DavidAnson are you able to reproduce this? If yes, shall we reopen this issue or create a new one?

[DavidAnson]

It may be a little while before I get a chance to look into this. It sounds like things worked correctly according to the previous scenario, but are breaking under different circumstances. Please open a new issue with the information above.