Skip to content

Resolver option cache doesn't take into account process.cwd() #217

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kingston opened this issue Mar 16, 2023 · 2 comments · Fixed by #219
Closed

Resolver option cache doesn't take into account process.cwd() #217

kingston opened this issue Mar 16, 2023 · 2 comments · Fixed by #219

Comments

@kingston
Copy link

In version 3.5.0 of the plugin, the comparison for the cached resolver options was updated to use the object hash instead of referential equality. However, this led to a subtle bug with monorepo setups in VSCode where the import resolver would cache the path map from one project for all projects resulting in continual errors for all other projects.

I went into the rabbit hole and figured out that it was because the initMappers function relies on process.cwd():


However, the logic for checking whether to initMappers only check options:
if (mappers && mappersCachedOptions === options) {
) and does not check if process.cwd() has changed as well. Therefore, the cache for mappers is never updated resulting in an error when the process.cwd() is changed. This is something that the VSCode ESLint plugin does to support multiple working directories (https://github.com/microsoft/vscode-eslint/blob/560df864580a9381d84b2f1e408f46f6600fda48/server/src/eslint.ts#L1056).

It seems a simple solution is to also check if process.cwd() has changed in initMappers. I can put together a quick PR for that if it seems like a reasonable solution.

@luke-layerhealth
Copy link
Contributor

luke-layerhealth commented Mar 29, 2023

@kingston I submitted a PR. linked above. I didn't do a deep dive into the code but if you want to double check that it does what you would expect that would be helpful. It's only two lines. Trying to push this through. If it doesn't match what you would expect. Feel free to comment and I'll fix it up, or you can submit your own and I'll close mine out.

@kingston
Copy link
Author

@kingston I submitted a PR. linked above. I didn't do a deep dive into the code but if you want to double check that it does what you would expect that would be helpful. It's only two lines. Trying to push this through. If it doesn't match what you would expect. Feel free to comment and I'll fix it up, or you can submit your own and I'll close mine out.

Oh awesome - looks great. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants