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

chore(deps): package-lock maintainance #4621

Conversation

pichlermarc
Copy link
Member

Updates dependencies in package-lock.json, first commit is actually #4620. This will be rebased once it's merged.

@trentm
Copy link
Contributor

trentm commented Apr 10, 2024

Whoa, a 41k line change to package-lock?

@pichlermarc
Copy link
Member Author

Ah, something must've gone wrong - I actually just ran npm update.
I'll look into it once we're done with #4620

@trentm
Copy link
Contributor

trentm commented Apr 10, 2024

I see that in #4601 part of the change to package-lock included this:

diff --git a/package-lock.json b/package-lock.json
index 6cc71665..503a0f15 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -10,7 +10,6 @@
       "hasInstallScript": true,
       "license": "Apache-2.0",
       "workspaces": [
-        "api",
         "packages/*",
         "experimental/packages/*",
         "experimental/examples/*",
        

I don't know that that is problematic, but it is weird.

@trentm
Copy link
Contributor

trentm commented Apr 10, 2024

package-lock and npm workspaces sure are a ride.

@pichlermarc
Copy link
Member Author

I don't know that that is problematic, but it is weird.

I think I know why this happens: It must be the prepare release script, we remove the API there so that lerna can do it's thing. We'll likely need to sync package-lock again after we do that.

@pichlermarc
Copy link
Member Author

I don't know that that is problematic, but it is weird.

I think I know why this happens: It must be the prepare release script, we remove the API there so that lerna can do it's thing. We'll likely need to sync package-lock again after we do that.

#4623 should take care of this.

@trentm
Copy link
Contributor

trentm commented Apr 11, 2024

I actually just ran npm update.

I tried that too, and my diff is 72k! :)

% git diff | wc -l
   72979

I don't really know what to say. I think the huge diff is from shuffling around where deps are installed. E.g. in my 72k line diff, all the mocha@10 deps were shuffled from the top-level node_modules/ to the separate .../node_modules/ dirs for each package... and the one usage of mocha@9 moved to the top-level.

I guess we just take it and move on.

@pichlermarc
Copy link
Member Author

I actually just ran npm update.

I tried that too, and my diff is 72k! :)

New highscore :D

% git diff | wc -l
   72979

I don't really know what to say. I think the huge diff is from shuffling around where deps are installed. E.g. in my 72k line diff, all the mocha@10 deps were shuffled from the top-level node_modules/ to the separate .../node_modules/ dirs for each package... and the one usage of mocha@9 moved to the top-level.

I guess we just take it and move on.

I'll try to enable packge-lock maintainance on renovate bot. Maybe it can manage to produce a diff that's a bit less daunting to look at. 🤔

@pichlermarc
Copy link
Member Author

PR to enable lock file maintenance, let's see what it does: #4628 🙂

@pichlermarc
Copy link
Member Author

Closing this for now as I think going the renovate route will be better.

@pichlermarc pichlermarc deleted the chore/package-lock-maintainance branch April 11, 2024 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants