chore: fix broken yarn lint script #6172

nazarhussain · 2023-12-08T13:52:49Z

Motivation

Lint the yarn warnings

Description

Lint yarn warnings.

Steps to test or reproduce

Run all scripts/tests.

nazarhussain · 2023-12-08T14:15:38Z

scripts/assert_no_yarn_warnings.sh

@@ -10,16 +10,16 @@ MATCH=("warning")
 # There are few yarn warnings we can't find a fix for. Excluding those. 
 # TODO: Keep checking occasionally if the warnings are fixed upstream.
 EXCLUDE=("Pattern \[\".*\"\] is trying to unpack in the same destination")


Could not find a solution to fix this warning, so why have to disable it from lint script.

Unmet peer dependency warnings seem to be resolved but there are further warnings. I quickly looked at how to resolve those warnings before, I linked to some issues here but does not seem like there are quick solutions for it.

[4/5] Linking dependencies... warning Workspaces can only be enabled in private projects. warning Workspaces can only be enabled in private projects.

Lint run did not properly catch the warnings, see job run.

Related to workspace warning, I am seeing it on Mac for long time. It's a known issue and non-fixable in yarn 1.

yarnpkg/yarn#8580 (comment)

The workspace warning only happens since #6074 and the lint script is not catching that currently. I haven't seen any yarn warnings since we added the script until now.

CI run before PR got merged (no warnings), see job run

1m 20s Run scripts/assert_no_yarn_warnings.sh yarn install v[1](https://github.com/ChainSafe/lodestar/actions/runs/7137600806/job/19437903275#step:5:1).22.21 [1/5] Validating package.json... [2/5] Resolving packages... [3/5] Fetching packages... [4/5] Linking dependencies... [5/5] Building fresh packages... Done in [8](https://github.com/ChainSafe/lodestar/actions/runs/7137600806/job/19437903275#step:5:9)0.00s. No warnings in yarn install --check-files

CI run on this branch, see job run

Run scripts/assert_no_yarn_warnings.sh yarn install v1.22.21 [1/5] Validating package.json... [2/5] Resolving packages... [3/5] Fetching packages... warning Pattern ["buffer@^6.0.3"] is trying to unpack in the same destination "/home/runner/.cache/yarn/v6/npm-buffer-polyfill-6.0.3-2ace5[7](https://github.com/ChainSafe/lodestar/actions/runs/7142943938/job/19453461545?pr=6172#step:5:8)[8](https://github.com/ChainSafe/lodestar/actions/runs/7142943938/job/19453461545?pr=6172#step:5:9)45[9](https://github.com/ChainSafe/lodestar/actions/runs/7142943938/job/19453461545?pr=6172#step:5:10)cc8fbe2a70aaa8f52ee63b6a74c6c6-integrity/node_modules/buffer-polyfill" as pattern ["buffer-polyfill@npm:buffer@^6.0.3"]. This could result in non-deterministic behavior, skipping. [4/5] Linking dependencies... warning Workspaces can only be enabled in private projects. warning Workspaces can only be enabled in private projects. [5/5] Building fresh packages... Done in 84.89s. Running 'grep -qi -e 'warning' | grep -qi -v -e 'Pattern \[".*"\] is trying to unpack in the same destination'' No warnings in yarn install --check-files

After further investigation, it turns out both warnings are related to https://github.com/davidmyersdev/vite-plugin-node-polyfills, which we use here

lodestar/vitest.base.browser.config.ts

Line 4 in 8bd19f4

import {nodePolyfills} from "vite-plugin-node-polyfills";

The warning warning Pattern ["buffer@^6.0.3"] is trying to unpack in the same destination "/home/nico/.cache/yarn/v6/npm-buffer-polyfill-6.0.3-2ace578459cc8fbe2a70aaa8f52ee63b6a74c6c6-integrity/node_modules/buffer-polyfill" as pattern ["buffer-polyfill@npm:buffer@^6.0.3"]. This could result in non-deterministic behavior, skipping. happens because of
https://github.com/davidmyersdev/vite-plugin-node-polyfills/blob/bfb529025d722a49cf98c395550fd46454d0de39/package.json#L73

And warning Workspaces can only be enabled in private projects. happens due to
https://github.com/davidmyersdev/vite-plugin-node-polyfills/blob/bfb529025d722a49cf98c395550fd46454d0de39/package.json#L94-L96

I haven't looked further on why we need those polyfills but shouldn't our code work in the browser without them? What if we run the code without polyfills in production will it break?

Have tried to remove vite-plugin-node-polyfills package but so far unsuccessful to make the browser tests work (unstable...nflaig/fix-yarn-warnings). Tried few different things but none worked, what I didn't try yet is using https://github.com/niksy/node-stdlib-browser#vite directly, this way we can avoid the polyfill package.

Opened two issues on polyfills package, maybe they can resolve it on their end

warning Workspaces can only be enabled in private projects. davidmyersdev/vite-plugin-node-polyfills#60

warning Pattern ["buffer@^6.0.3"] is trying to unpack in the same destination davidmyersdev/vite-plugin-node-polyfills#61

We need to use the polyfills for the browsers as Nodejs modules are not available in browser environment.

We can try few other polyfill packages, but using browsified modules directly meant to have multiple dependencies a a lot of vite configuration to manage in our code.

codecov · 2023-12-08T15:17:54Z

Codecov Report

Merging #6172 (1ca486f) into unstable (8bd19f4) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff            @@
##           unstable    #6172   +/-   ##
=========================================
  Coverage     80.02%   80.02%           
=========================================
  Files            19       19           
  Lines          1717     1717           
  Branches        155      155           
=========================================
  Hits           1374     1374           
  Misses          341      341           
  Partials          2        2

wemeetagain · 2023-12-08T16:20:00Z

package.json

@@ -107,6 +107,7 @@
  "resolutions": {
    "dns-over-http-resolver": "^2.1.1",
    "chai": "^4.3.10",
-    "loupe": "^2.3.6"
+    "loupe": "^2.3.6",
+    "vite": "^5.0.7"


is this related to the PR description?

Yes it's related. Used this change to fix few unmet dependencies yarn warnings while fixing and testing the yarn lint script.

nflaig

scripts/assert_no_yarn_warnings.sh is broken

nazarhussain · 2023-12-08T17:12:33Z

scripts/assert_no_yarn_warnings.sh is broken

It seems to be working on mac but not on the CI, as the output on the CI shows that it concatenated yarn output in one single line.

While on the Mac it preserves the output.

Need to figure out why CI is skipping the line breaks.

nflaig · 2023-12-09T10:21:23Z

@nazarhussain let's close this PR and instead try to get rid of the actual warnings, this way we avoid wasting time on the script and resolve the actual problem. I think we can keep the vite-plugin-node-polyfills package, just need some cooperation from the author to get rid of the workspaces setting in their package (this is hostile towards any yarn project) and see if we can also resolve the buffer warning, there is also a workaround we could use, see davidmyersdev/vite-plugin-node-polyfills#61 (comment).

nflaig · 2023-12-23T11:47:54Z

Closed in favor of #6232

nazarhussain requested a review from a team as a code owner December 8, 2023 13:52

nazarhussain self-assigned this Dec 8, 2023

nazarhussain added 2 commits December 8, 2023 15:07

Fix yarn lint script

b93d9c4

Update the lock file

bb57c5b

nazarhussain force-pushed the nh/yarn-lint-fix branch from c1d2fb8 to bb57c5b Compare December 8, 2023 14:07

nazarhussain commented Dec 8, 2023

View reviewed changes

nflaig changed the title ~~fix: fix broken yarn lint script~~ chore: fix broken yarn lint script Dec 8, 2023

nazarhussain added 2 commits December 8, 2023 15:40

Update the lint script

69d6550

Fix warnings

1ca486f

wemeetagain reviewed Dec 8, 2023

View reviewed changes

nflaig requested changes Dec 8, 2023

View reviewed changes

nflaig closed this Dec 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: fix broken yarn lint script #6172

chore: fix broken yarn lint script #6172

nazarhussain commented Dec 8, 2023

nazarhussain Dec 8, 2023

nflaig Dec 8, 2023

nazarhussain Dec 8, 2023 •

edited

Loading

nflaig Dec 8, 2023 •

edited

Loading

nflaig Dec 8, 2023 •

edited

Loading

nflaig Dec 8, 2023

nazarhussain Dec 8, 2023

codecov bot commented Dec 8, 2023

wemeetagain Dec 8, 2023

nazarhussain Dec 8, 2023

nflaig left a comment

nazarhussain commented Dec 8, 2023

nflaig commented Dec 9, 2023

nflaig commented Dec 23, 2023

chore: fix broken yarn lint script #6172

chore: fix broken yarn lint script #6172

Conversation

nazarhussain commented Dec 8, 2023

nazarhussain Dec 8, 2023

Choose a reason for hiding this comment

nflaig Dec 8, 2023

Choose a reason for hiding this comment

nazarhussain Dec 8, 2023 • edited Loading

Choose a reason for hiding this comment

nflaig Dec 8, 2023 • edited Loading

Choose a reason for hiding this comment

nflaig Dec 8, 2023 • edited Loading

Choose a reason for hiding this comment

nflaig Dec 8, 2023

Choose a reason for hiding this comment

nazarhussain Dec 8, 2023

Choose a reason for hiding this comment

codecov bot commented Dec 8, 2023

Codecov Report

wemeetagain Dec 8, 2023

Choose a reason for hiding this comment

nazarhussain Dec 8, 2023

Choose a reason for hiding this comment

nflaig left a comment

Choose a reason for hiding this comment

nazarhussain commented Dec 8, 2023

nflaig commented Dec 9, 2023

nflaig commented Dec 23, 2023

nazarhussain Dec 8, 2023 •

edited

Loading

nflaig Dec 8, 2023 •

edited

Loading

nflaig Dec 8, 2023 •

edited

Loading