Skip to content

[PAY-3166] Updates ESLint dependencies to fix broken IDE experience#9146

Merged
schottra merged 6 commits into
mainfrom
update-eslint
Jul 17, 2024
Merged

[PAY-3166] Updates ESLint dependencies to fix broken IDE experience#9146
schottra merged 6 commits into
mainfrom
update-eslint

Conversation

@schottra

Copy link
Copy Markdown
Contributor

Description

(Side note: it's funny to me that a day and a half of effort resulted in such a small changeset...)

The ESLint extension has been randomly not working for some time. This is due to a bug in said extension that causes it to stop working when it encounters a fatal error in parsing configurations.

While investigating, I noticed that some of our workspaces/packages had older versions of ESLint installed in their local node_modules folder. These older versions did not support some of the options in our shared config file and would exit and brick the extension.

These older versions continued to stick around despite updating package.json files for said packages to the correct version. (8.56.0 being the common version across our packages). The cause here was leftover entries in package-lock.json that were not removed during hoisting. The flow is roughly as follows:

  1. Root directory has a dependency on eslint@8.56.0 and identity-service depends on eslint@7.x.
  2. Version 8.56.0 installed at node_modules/eslint and version 7.x installed at packages/identity-service/node_modules/eslint. T
  3. The package-lock.json file will have an entry for each of these.
  4. identity-service/package.json is updated to upgrade the dependency to version 8.56.0 either through manual editing of the file or through npm i --save-dev --save-exact -w identity-service eslint@8.56.0.
  5. package-lock.json is not updated to remove the entry for packages/identity-service/node_modules/eslint
  6. Subsequent installations via npm i at any level of the repository will read the wrong version from package-lock.json and install version 7.x into packages/identity-service/node_modules/eslint
  7. Extension starts at the currently opened file and traverses upward to look for a valid eslint binary and finds the v7.x version in packages/identity-service/node_modules
  8. Chaos ensues.

The crux of the issue is that the entry for packages/identity-service/node_modules/eslint will not be removed without explicitly uninstalling the module from the identity-service workspace (i.e npm rm -w identity-service eslint) and then re-installing the correct version (npm i --save-dev --save-exact -w identity-service eslint@8.56.0). During installation, when the version matches the version at the root, the package is properly hoisted/deduped and we get a single entry in package-lock.json for node_modules/eslint pointing to the shared version.

So!

With all that preamble out of the way...

  • I've followed the above procedure for uninstalling/reinstalling eslint for most workspace packages to normalize us on 8.56.0.
  • Additionally, I updated any plugin/config packages for eslint/prettier/standard to match those in eslint-config-audius if they were older. The ddex workspaces actually use newer versions, so I didn't want to downgrade them. The goal here was to update any plugins which referenced old incompatible versions so that npm ls --workspaces --include-workspace-root eslint shows a clean log of eslint versions with no "invalid" entries.
  • I did not fix any ESLint errors that popped up in services as a result of getting linting working again. I will fix any that prevent CI from merging cleanly.

The biggest offender for old packages was identity

Some future work we should do:

  • Migrate all of our packages to reference/extend the shared Audius eslint config (as a peer dep?) instead of installing their own versions of all of the config/plugin packages and then partially duplicating the setup from the shared config.
  • Fix a few "invalid" eslint plugin packages such as eslint-config-jest which are on incompatible versions in a few of the workspaces.
  • Update all eslint to use the version the ddex packages are on (9.1.0)
  • Write and run a script to help us find and remove any other orphaned hoisted dependencies!

How Has This Been Tested?

  1. Wiped and reinstalled node_modules after the change
  2. Restarted VSCode
  3. Opened a js/ts file in every workspace and examined the ESLint output to make sure it finished linting without any config errors
  4. npm run verify to ensure linting still works correctly

@changeset-bot

changeset-bot Bot commented Jul 16, 2024

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 72ec807

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

Comment thread package-lock.json

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The majority of the important change is here. But since lockfiles are always huge, I doubt you will be able to see it :-(

@rickyrombo rickyrombo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you!!!! 🚀 🚀 🚀

@sliptype

Copy link
Copy Markdown
Contributor

Migrate all of our packages to reference/extend the shared Audius eslint config (as a peer dep?) instead of installing their own versions of all of the config/plugin packages and then partially duplicating the setup from the shared config.

It shouldn't need to be a peer dep, we can just install the latest version of eslint-config-audius in every package that needs it and when we bump the version via changesets it will update in all the packages

@sliptype sliptype left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Amazing! Thank you for getting to the bottom of this 🙏

@schottra schottra merged commit dc387b0 into main Jul 17, 2024
@schottra schottra deleted the update-eslint branch July 17, 2024 14:33
audius-infra pushed a commit that referenced this pull request Jul 17, 2024
[ffd2ad3] [PAY-2783] Add filters to manager endpoints (#9109) Randy Schott
[dc387b0] [PAY-3166] Updates ESLint dependencies to fix broken IDE experience (#9146) Randy Schott
[bda6e63] Specifiers should be strings in challenge notifs (#9148) Raymond Jacobson
[96efa17] Cycle solana RPC connections better (#9147) Raymond Jacobson
[3d5ff3a] [C-4777] Prevent public track with future release date (#9139) Reed
[faf7908] Final touches to UDR (#9144) Raymond Jacobson
[15b031c] PROTO-1781: move verified notifs into pedalboard + ci (#9130) alecsavvy
[8d442d3] Fix misc UDR bugs (#9141) Raymond Jacobson
[1183296] PROTO-1879: rm PUT /user/email route (#9089) alecsavvy
audius-infra pushed a commit that referenced this pull request Jul 20, 2024
[0c89479] [PAY-3174][PAY-3179] Do not show confirmation modals for edit access during upload (#9204) Saliou Diallo
[a88b39b] [C-4493] Don't show deleted track rows in collections (#9209) Andrew Mendelsohn
[8712eb8] [C-4831] Populate AudiusQueryContext in fetchSaga (#9203) Andrew Mendelsohn
[69d6c0c] [C-4851] Update artwork and metadata fields layout (#9207) Andrew Mendelsohn
[e77abfb] Improve loading and debounce logic for advanced search tracks (#9206) Sebastian Klingler
[39ad581] [C-4849] Fullwidth AI attribution dropdown (#9205) Andrew Mendelsohn
[05db4bc] [PAY-3181][PAY-3182] Remove hidden tracks from trending (#9202) Saliou Diallo
[f034349] [QA-1461] Fix search v1 tag search (#9201) Dylan Jeffers
[237aa88] [C-4698] Route to solana play store in rate-cta-drawer (#9198) Dylan Jeffers
[65aca0b] [C-4698] Add solana dapp store update link (#9197) Dylan Jeffers
[78f501e] [C-4813 C-4814] Update release date and scheduled release (#9188) Dylan Jeffers
[790a30f] [PAY-3183] Fix mobile track header and special access box (#9187) Raymond Jacobson
[48ba3a0] [C-4804] Improve search performance pt 2 (#9185) Sebastian Klingler
[d5e5f83] [QA-1457] Fix purchase tab (#9191) Dylan Jeffers
[4bff092] Undo web filter button sorting (#9190) Kyle Shanks
[e07093e] [QA-1456] Hide purchase button and stats on tombstoned track rows (#9183) Andrew Mendelsohn
[b60693d] [C-4797] Fix BPM screen scroll clipping (#9180) Kyle Shanks
[e1b107a] [C-4740] Update loading state for new search screen track results (#9176) Kyle Shanks
[2534f81] [QA-1453] Big user-list images (#9182) Dylan Jeffers
[c6e607c] [C-4677] Include albums and playlists in tag search (#9179) Sebastian Klingler
[41c6011] [C-4692] Fix filter button option active help text color (#9181) Kyle Shanks
[86638a5] [PAY-3173] Update native mobile library for locked and hidden tracks (#9177) Saliou Diallo
[28cfba5] [QA-1451] Fix track tile action buttons (#9178) Dylan Jeffers
[9e542c2] [C-4744] Change clear recent search button to plain button (#9175) Kyle Shanks
[ca05312] [PAY-3163] Update editable access in native mobile (#9174) Saliou Diallo
[dba2775] Remove description dupe (#9173) Saliou Diallo
[a106da4] [C-4810, C-4801, C-4790, PAY-3171] Fix publish album tracks behavior; update confirmation modals (#9172) Andrew Mendelsohn
[418ecf2] [C-4803] Add mobile Price and Audience Field (#9168) Dylan Jeffers
[d236600] Fix mobile upload bpm requirement (#9171) Dylan Jeffers
[6a13fa3] [C-4675] [C-4665] [C-4736] [C-4732] [C-4737] [C-4734] [C-4742] Advanced search UI qa fixes part 2 (#9170) Sebastian Klingler
[3ec2e15] [C-4757] Add simple sorting for filter buttons with values (#9165) Kyle Shanks
[f861a9a] [C-4672] [C-4745] [C-4807] Advanced search UI qa fixes (#9167) Sebastian Klingler
[8d18c27] [C-4788] Update filter button to fix toggle filter button functionality (#9164) Kyle Shanks
[2f19663] [C-4805] Fix track filters (#9159) Sebastian Klingler
[2a2aeeb] Fix usage of missing SDK function for Manager Mode reject flow (#9163) Randy Schott
[b07590c] [C-4671] Add clear search button (#9162) Sebastian Klingler
[ff4c649] [C-4789] Update web key and bpm filter buttons to respect the filter value (#9161) Kyle Shanks
[79f0e5e] [PAY-3168] Remove SDK shadowing (#9157) Randy Schott
[818c73b] [C-4787] Fix advanced search sort dropdowns (#9158) Sebastian Klingler
[45c93fb] [PAY-3148] Add better support for gated tracks in library (#9156) Saliou Diallo
[86d9b7a] Add UTM tracking in amplitude (#9153) Isaac Solo
[ffd2ad3] [PAY-2783] Add filters to manager endpoints (#9109) Randy Schott
[dc387b0] [PAY-3166] Updates ESLint dependencies to fix broken IDE experience (#9146) Randy Schott
[d68b57e] [C-4796] Mobile Visibility Field (#9152) Dylan Jeffers
[6c9e2b5] Fix type error (#9151) Sebastian Klingler
[7246503] [QA-1429] Fix avatar sizing (#9154) Dylan Jeffers
[28de67e] [QA-1443] Remove faulty download all logic (#9149) Raymond Jacobson
[139bb9c] [C-4794] Hide social actions on now playing drawer for hidden tracks (#9150) Reed
[02b75bb] [C-4668] Improve web search v2 perf and structure (#9134) Sebastian Klingler
[3d5ff3a] [C-4777] Prevent public track with future release date (#9139) Reed
[12becab] [C-4719] Fix local collections selector (#9145) Andrew Mendelsohn
[4be6e57] [PAY-3164] Allow editing access for download-only gated tracks (#9143) Saliou Diallo
[c07106d] [C-4772] Update mobile filter buttons to respect filter value (#9142) Kyle Shanks
[7794808] [QA-964] Fix too many suggested artists (#9138) Raymond Jacobson
[b97c0fc] Fix offline mode after fast load change (#9137) Raymond Jacobson
[83020e0] [C-4709] Fix modals on mobile-web (#9136) Reed
[4a635ca] [C-4778, C-4786] Update visibility change confirmation (#9135) Andrew Mendelsohn
[46074ae] Fix account playlists entering cache and caching of currentWeb3User (#9131) Raymond Jacobson
[97ae887] Filter out searchv2 history items for search v1 (#9127) Sebastian Klingler
[1d64f8c] [C-4776] Fix edit collection submit (#9126) Dylan Jeffers
[ec16b61] [C-4771] Hide stats in TracksTable for hidden tracks (#9125) Reed
[0f62cc0] Fix slow startup (#9121) Raymond Jacobson
[3f04084] Bump mobile apps to 107 (#9124) Dylan Jeffers
[3c055df] [PAY-3149] Edit track confirmation modal when listeners may lose access (#9107) Saliou Diallo
[b3a6b89] Fix web lint and permalink passing (#9122) Raymond Jacobson
[aa64792] Fix misc component issues on startup (#9118) Raymond Jacobson
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants