Skip to content
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

Add fsevents as optional dependency to public package #5503

Merged
merged 2 commits into from
Jul 16, 2023

Conversation

adampauls
Copy link
Contributor

I believe this should resolve #5415, but I'm not an expert here. My understanding is that the published pyright package does the npm equivalent of statically linking the chokidar dependency, which means that fsevents is not pulled in if that linking is done on a machine that is not run on MacOS.

This setup is not ideal since the version could drift from the one expected the chokidar embedded in pyright, but I think it's probably okay.

@erictraut
Copy link
Collaborator

Please don't submit PRs if you're unsure of their effects or don't fully understand the problem. Issue #5415 requires deeper investigation. I'll get to that when I have a chance.

@erictraut erictraut closed this Jul 14, 2023
@erictraut
Copy link
Collaborator

Since you're able to repro this issue on your machine, does this change make the problem go away?

@adampauls
Copy link
Contributor Author

Yes. I tested by running npm run install:all && npm run build:cli:dev, then doing a local npm install /path/to/local/pyright in the .cache directory. I verified that repeating those steps without the change caused the error.

@erictraut
Copy link
Collaborator

OK, that's pretty good evidence that this is a proper diagnosis and an effective fix. I'll merge your PR. Thanks!

@erictraut erictraut reopened this Jul 16, 2023
@erictraut erictraut merged commit cc92429 into microsoft:main Jul 16, 2023
2 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants