feat: Add npm support for prune #2203

chris-olszewski · 2022-10-11T20:38:43Z

This PR adds a lockfile implementation for npm's package-lock.json allowing us to properly support pruning npm based repos.

Addresses #1830

Notes:

Unlike other lockfile implementations the return value of AllDependencies returns fully resolved lockfile keys and not just package names. This is necessary since a package's resolved version might be nested e.g. node_modules/foo/node_modules/bar and calls to ResolveDependency doesn't include who is depending on the package.
This implementation does not aim to produce a byte perfect lockfile. By default package-lock.json includes every field that appears any package.json in the dep graph. The fields that are not copied over should be limited to package metadata.
Due to time constraints this doesn't support lockfiles created by npm 6 (Node 14). Hopefully this won't be a large chunk of users since that version is hitting EOL this April.

vercel · 2022-10-11T20:38:47Z

@chris-olszewski is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

vercel · 2022-10-11T20:38:49Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
examples-basic-web	🔄 Building (Inspect)	Visit Preview	Oct 13, 2022 at 6:52PM (UTC)

4 Ignored Deployments

Name	Status	Preview	Updated
examples-designsystem-docs	⬜️ Ignored (Inspect)	Visit Preview	Oct 13, 2022 at 6:52PM (UTC)
examples-kitchensink-blog	⬜️ Ignored (Inspect)	Visit Preview	Oct 13, 2022 at 6:52PM (UTC)
examples-native-web	⬜️ Ignored (Inspect)	Visit Preview	Oct 13, 2022 at 6:52PM (UTC)
examples-svelte-web	⬜️ Ignored (Inspect)	Visit Preview	Oct 13, 2022 at 6:52PM (UTC)

tknickman · 2022-10-13T17:24:37Z

cli/scripts/monorepo.ts

@@ -356,7 +356,7 @@ fs.copyFileSync(
          ...options,
        });
      case "npm":
-        return execa.sync("npm", ["run", command, ...(args || [])], {
+        return execa.sync("npm", [command, ...(args || [])], {


Curious about this one, don't we still need run for npm?

I haven't dug through the git history, but I assume that this function was originally used to run package scripts in tests. Currently in our e2e tests we only use it to run install commands and script execution uses Monorepo.turbo

cli/integration_tests/lockfile_aware_caching/npm.t

cli/internal/lockfile/npm_lockfile.go

mehulkar

Some very minor comments. I'm a bit handicapped in the review here, because I haven't spent a great deal of time looking at what goes in the npm lockfile, but the code structure and implementation generally looks good to me!

mehulkar · 2022-10-13T17:30:59Z

cli/scripts/e2e/e2e.ts

    // Test `turbo prune --scope=a`
    // @todo refactor with other package managers
-    const installArgs =
-      npmClient === "berry" ? ["--immutable"] : ["--frozen-lockfile"];
+    const [installCmd, ...installArgs] =


could change return value to match { installCmd, installArgs }, but not a big deal

mehulkar · 2022-10-13T17:38:13Z

cli/internal/lockfile/npm_lockfile.go

+	if !ok {
+		return nil, false
+	}
+	deps := make(map[string]string, len(entry.Dependencies)+len(entry.DevDependencies)+len(entry.PeerDependencies)+len(entry.OptionalDependencies))


go fmt doesn't like spaces between + operators?

Apparently not?

cli/internal/lockfile/npm_lockfile.go

chris-olszewski added 3 commits October 11, 2022 13:18

add npm lockfile impl

4401d79

link up npm lockfile to npm package manager impl

7de5899

fixups for internal packages and adding npm prune e2e tests

cbd0f66

toggle npm canPrune expected value

8266d3c

chris-olszewski marked this pull request as ready for review October 11, 2022 22:40

chris-olszewski requested a review from a team as a code owner October 11, 2022 22:40

chris-olszewski requested review from mehulkar and tknickman October 11, 2022 22:40

chris-olszewski added 3 commits October 12, 2022 15:53

Merge branch 'main' into add_npm_lockfile_impl

73df153

update npm lockfile aware cache test

69f34d6

Merge branch 'main' into add_npm_lockfile_impl

8b0ba67

tknickman approved these changes Oct 13, 2022

View reviewed changes

mehulkar approved these changes Oct 13, 2022

View reviewed changes

Merge branch 'main' into add_npm_lockfile_impl

ff7976a

vercel bot deployed to Preview – examples-svelte-web October 13, 2022 18:23 View deployment

vercel bot deployed to Preview – examples-basic-web October 13, 2022 18:23 View deployment

vercel bot deployed to Preview – examples-native-web October 13, 2022 18:23 View deployment

vercel bot deployed to Preview – examples-designsystem-docs October 13, 2022 18:23 View deployment

vercel bot deployed to Preview – examples-kitchensink-blog October 13, 2022 18:23 View deployment

chris-olszewski added 2 commits October 13, 2022 11:35

fixup integration test title and comment

c0ba982

update monorepo error integration tests since hashes change

8b212ba

chris-olszewski merged commit e4ef973 into vercel:main Oct 13, 2022

chris-olszewski mentioned this pull request Oct 18, 2022

Update deploying-with-docker.mdx to indicate it does not work with npm #2262

Closed

jaredpalmer mentioned this pull request Nov 16, 2022

docs: Remove callout now that we support prune w/npm #2725

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add npm support for prune #2203

feat: Add npm support for prune #2203

chris-olszewski commented Oct 11, 2022

vercel bot commented Oct 11, 2022

vercel bot commented Oct 11, 2022 •

edited

Loading

tknickman Oct 13, 2022

chris-olszewski Oct 13, 2022

mehulkar left a comment

mehulkar Oct 13, 2022

mehulkar Oct 13, 2022

chris-olszewski Oct 13, 2022

feat: Add npm support for prune #2203

feat: Add npm support for prune #2203

Conversation

chris-olszewski commented Oct 11, 2022

vercel bot commented Oct 11, 2022

vercel bot commented Oct 11, 2022 • edited Loading

tknickman Oct 13, 2022

Choose a reason for hiding this comment

chris-olszewski Oct 13, 2022

Choose a reason for hiding this comment

mehulkar left a comment

Choose a reason for hiding this comment

mehulkar Oct 13, 2022

Choose a reason for hiding this comment

mehulkar Oct 13, 2022

Choose a reason for hiding this comment

chris-olszewski Oct 13, 2022

Choose a reason for hiding this comment

vercel bot commented Oct 11, 2022 •

edited

Loading