Skip to content
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

Upgrade npm dependencies #119

Merged
merged 2 commits into from
May 3, 2023
Merged

Upgrade npm dependencies #119

merged 2 commits into from
May 3, 2023

Conversation

timstallmann
Copy link
Collaborator

No description provided.

@timstallmann
Copy link
Collaborator Author

@Bogidon any intuition as to why this run of npm upgrade is all of a sudden causing tests to fail?

@Bogidon
Copy link
Contributor

Bogidon commented May 2, 2023

@timstallmann it looks like the culprit is vite@4.3.x. This PR upgrades vite 4.1.1 -> 4.3.2. I tested 4.2.2 and it works. This is the package-lock.json diff of upgrading vite 4.2.2 -> 4.3.2:

image

The tests are failing because the mocks are no longer working for the fs module, which is a core node module. Looking at the package-lock diff, the fact that resolve and its dependency is-core-module was removed seems like a likely suspect to me. I traced it back to this PR in vite: vitejs/vite#12770

I tested the latest vite (4.3.4) too and this issue persists. Not seeing other folks having this issue after some quick searching, but the release is still quite new.

A few possible suggestions:

(1) Either we fix this in a semi-permanent fashion by locking the vite version to 4.2.x by changing the version requirement in the package.json. npm update would not attempt to upgrade past this in the future, so we'd have to remember to review this in the future:

-        "vite": "^4.0.4",
+        "vite": "~4.2.2",

(2) You can just run this command on top of this PR's branch: npm install vite@4.2.2 and this will change just the package-lock.json to 4.2.2. Commit and push that. The next time you run npm update it will again try to update to whatever the latest 4.x is, which would either alert us of the error if it persists or upgrade successfully. I think I like this option the best. Especially since JSON does not support comments so it would be difficult to remember to try upgrading past 4.2.x if taking option 1.

(3) I could figure out exactly what's wrong and either patch vite or figure out a different mocking mechanism for fs. This would be significant time spend however.

@timstallmann
Copy link
Collaborator Author

@Bogidon thanks for sleuthing that out! I'm pinning Vite to version 4.2.x for the time being, and we can re-evaluate next time a major version upgrade cycle comes around around on the maintenance checklist (in 3 months)..

@timstallmann timstallmann merged commit ca22fb8 into develop May 3, 2023
@timstallmann timstallmann deleted the upgrade-dependencies-2 branch May 3, 2023 00: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.

2 participants