Skip to content

Surface error occurring during import of @parcel/watcher #9580

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

Merged
merged 7 commits into from
Oct 12, 2023
Merged

Surface error occurring during import of @parcel/watcher #9580

merged 7 commits into from
Oct 12, 2023

Conversation

spawnia
Copy link
Contributor

@spawnia spawnia commented Aug 2, 2023

Description

Follow up from #9506, which provides the following error message when failing to import @parcel/watcher:

$ graphql-codegen --project=limes --watch
✔ Parse Configuration
✔ Generate outputs
  Parcel watcher not found. To use this feature, please make sure to provide @parcel/watcher as a peer dependency.

I think the provided error message is unclear and misleading in a couple of ways:

  1. It suggests to provide @parcel/watcher as a peer dependency. It makes sense for it to be a peer dependency of this package, but unless the user itself is distributing a package, they should just install the package.
  2. It swallows any underlying error during the import that could prevent @parcel/watcher from working, even if installed.

In my environment, simply installing @parcel/watcher as a dependency was not sufficient. After some debugging, I modified the piece of code that produces the error message to append the thrown err and found that the cause was more detailed:

$ graphql-codegen --project=limes --watch
✔ Parse Configuration
✔ Generate outputs
  Parcel watcher not found. To use this feature, please make sure to provide @parcel/watcher as a peer dependency. Error: No prebuild or local build of @parcel/watcher found. Tried @parcel/watcher-linux-x64-glibc. 
Please ensure it is installed (don't use --no-optional when installing with npm). Otherwise it is possible we don't support your platform yet. If this is the case, please report an issue to https://github.com/parce
l-bundler/watcher.

Thus, I am proposing this change to the error message which improves upon the previous mentioned points:

  1. It suggest that @parcel/watcher must be installed, not just a peer dependency.
  2. It includes the error thrown during import to allow further diagnosis of the issue.

The final output is now this:

$ graphql-codegen --project=limes --watch
✔ Parse Configuration
✔ Generate outputs
  Failed to import @parcel/watcher due to the following error (to use watch mode, install https://www.npmjs.com/package/@parcel/watcher):
Error: No prebuild or local build of @parcel/watcher found. Tried @parcel/watcher-linux-x64-glibc. Please ensure it is installed (don't use --no-optional when installing with npm). Otherwise it is possible we don't
 support your platform yet. If this is the case, please report an issue to https://github.com/parcel-bundler/watcher.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Copy-paste the updated error message in my projects node_modules/@graphql-codegen/cli/cjs/utils/watcher.js
  • Rerun the failing graphql-codegen --watch command

Test Environment:

  • Docker Image: node:18.13.0-bullseye-slim
  • @graphql-codegen/cli: 5.0.0

Checklist:

  • I have followed the CONTRIBUTING doc and the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@changeset-bot
Copy link

changeset-bot bot commented Aug 2, 2023

🦋 Changeset detected

Latest commit: 8061d85

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@graphql-codegen/cli Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@saihaj
Copy link
Collaborator

saihaj commented Aug 25, 2023

@spawnia can you please run yarn prettier . --write and commit the changes?

@spawnia
Copy link
Contributor Author

spawnia commented Aug 26, 2023

Sure. I found no mention of this in the contributor guide, perhaps something like https://github.com/marketplace/actions/prettier-action could help to smooth this process.

@spawnia
Copy link
Contributor Author

spawnia commented Oct 11, 2023

How can the failing PR check be solved? I would love for this change to be included.

Copy link
Collaborator

@saihaj saihaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for the delay on this one

@saihaj saihaj merged commit 50253b1 into dotansimha:master Oct 12, 2023
@spawnia spawnia deleted the surface-errors-during-parcel-watcher-import branch October 12, 2023 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants