Fix yarn and eslint warnings #1597
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Merged
robknight
force-pushed
the
rob/fix-yarn-warnings
branch
from
8f39874
to
ea84a86
Compare
artwyman
added a commit
that referenced
this pull request
Mar 18, 2024
This should make sure these new packages don't get missed in the bulk update in #1597.
robknight
force-pushed
the
rob/fix-yarn-warnings
branch
from
ea84a86
to
498dd30
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes https://linear.app/0xparc-pcd/issue/0XP-238
Closes https://linear.app/0xparc-pcd/issue/0XP-239
We have various warnings on
yarn install, mostly relating to missing or incorrect peer dependencies. Some of these were because of mismatches between expected package versions and the installed versions, and were solved by upgrading the dependant packages so that the dependencies match their expected version ranges (mostly upgradingeslintto 8.x).In addition, various
@opentelemetrypackages had mismatches in their dependency versions, which has been solved by pinning to specific compatible versions rather than allowing yarn to install newer but potentially incompatible dependencies.In other cases, it was necessary to install packages as
devDependenciesso that the peer package would stop complaining about a missingpeerDependency. This is odd, as no functional issues seemed to arise from this, but it seems like the correct thing to do is to install the missing dependency.As a result of upgrading
eslint, the fact thatanon-message-clientlacks aneslintrc.jsor equivalent config file became a problem, asyarn lintwould hang while prompting the user to decide what to do about this. So, I added a configuration file, which means thatyarn lintnow actually runs foranon-message-client. This threw up some simple linting issues (e.g. missing function return types) which I fixed.Finally, I took the opportunity to make the version number explicit on the
@pcd/eslint-config-customand@pcd/tsconfigdependencies throughout the codebase. Previously we were generally using'*'as the version number, but this was flagged as an issue here.The result is that most of the warnings have gone away. The following ones remain:
The incorrect peer dependency for
react-qr-readercomes from the fact thatreact-qr-readerexpects a maximum React version of^17.0.0, and we are on18.2.x. There is an unmerged PR for this in thereact-qr-readerrepo from November, which looks like it would resolve the issue if merged: JodusNodus/react-qr-reader#361