Skip to content
This repository was archived by the owner on Sep 17, 2023. It is now read-only.

fix: improve error messages presented to the user #191

Merged
merged 1 commit into from
Sep 29, 2022

Conversation

EricCrosson
Copy link
Contributor

This commit improves error messages presented to the user when encountering errors parsing JSON.

This required a bit of refactoring, since we were performing too much work up-front, and swallowing the helpful errors with a fallback (that stops looking for a lerna manifest and starts looking for an npm 7 workspace) that happened too late.

I took this opportunity to improve the representation of package.json manifests, which makes interaction with serde more ergonomic.

Here's an example of an error message when I remove the version property in a package.json file:

; monorepo pin --check
Error: Unexpected contents in "./packages/my-package/package.json"

I'm trying to parse the following properties and values out
of this package.json file:

- name: string
- version: string

and if the any of the following values are present, I expect
them to be a JSON object with string keys and string values:

- dependencies
- devDependencies
- optionalDependencies
- peerDependencies


Caused by:
    0: Unable to parse JSON from file '"./packages/my-package/package.json"'
    1: missing field `version` at line 103 column 1

cc @lifeiscontent

This commit improves error messages presented to the user when
encountering errors parsing JSON.

This required a bit of refactoring, since we were performing too much
work up-front, and swallowing the helpful errors with a fallback (that
stops looking for a lerna manifest and starts looking for an npm 7
workspace) that happened too late.

I took this opportunity to improve the representation of package.json
manifests, which makes interaction with serde more ergonomic.
- name: string
- version: string

and if the any of the following values are present, I expect

Choose a reason for hiding this comment

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

@EricCrosson extraneous the here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hot dog @louib-bitgo you are on fire catching these typos! 🌭 🌭 🌭

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in #192

Copy link
Contributor

@ocornoc ocornoc left a comment

Choose a reason for hiding this comment

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

I don't have any unique criticism, looks good!

@EricCrosson EricCrosson merged commit f1742b4 into master Sep 29, 2022
@EricCrosson EricCrosson deleted the improve-error-messages branch September 29, 2022 19:10
@github-actions
Copy link

🎉 This PR is included in version 4.2.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

3 participants