fix(arborist): don't fetch packuments for uninstallable optional peer deps

[danielroe]

this is a PR aimed to skip fetching packuments for optional peers that won't be installed. this can be extremely costly.

for example, with an empty cache, npm install vite into an empty project installs 10 packages but fetches full packuments for playwright (16.6MB), @types/node (11MB), webdriverio, jsdom, sass, less, terser, and (via vite's optional peer on vitest) all of vitest's optional peers too:

	before	after
registry requests	86	54
cached registry data	~121MB	~72MB

loadPeerSet resolves unmet optional peer edges into the virtual root, even though they won't be used and in fact be pruned later on

the fix is very simple; we just skip peerOptional edges if they have no current resolution and no parent edge. if something else ends up installing the package, the edge is still validated at placement time.

having said all of that, I'm a relative newcomer to the npm cli, so do let me know if there's a reason for this fetching that I'm missing 🙏

Note

the nest when peerOptional conflicts snapshot did change shape.

imo it was a bug before - the unmet optional peer was eagerly placed and 'won' the top-level slot, forcing the required peer chain to nest, but now the required chain wins and the optional subtree nests instead.

both before/after were valid trees (two copies of the conflicted peer, all edges valid, etc). I've added assertions for this, but it's worth noting that installs with conflicting optional peers can produce a differently shaped node_modules than before.

(ht to @pi0, who spotted this)

References

Fixes#9876

[jakebailey]

I also saw the Bluesky post and independently found the same fix. This is a lot more meaningful now that TypeScript has optional peer deps for platform specific binaries.

[martinrrm]

Thanks for addressing the comments, LGTM!

[danielroe]

not at all! thank you for the very thorough and helpful review. ❤️

[github-actions]

🎉 Backport to release/v11 created: #9903

[danielroe]

this is a PR aimed to skip fetching packuments for optional peers that won't be installed. this can be extremely costly.

for example, with an empty cache, npm install vite into an empty project installs 10 packages but fetches full packuments for playwright (16.6MB), @types/node (11MB), webdriverio, jsdom, sass, less, terser, and (via vite's optional peer on vitest) all of vitest's optional peers too:

	before	after
registry requests	86	54
cached registry data	~121MB	~72MB

loadPeerSet resolves unmet optional peer edges into the virtual root, even though they won't be used and in fact be pruned later on

the fix is very simple; we just skip peerOptional edges if they have no current resolution and no parent edge. if something else ends up installing the package, the edge is still validated at placement time.

having said all of that, I'm a relative newcomer to the npm cli, so do let me know if there's a reason for this fetching that I'm missing 🙏

Note

the nest when peerOptional conflicts snapshot did change shape.

imo it was a bug before - the unmet optional peer was eagerly placed and 'won' the top-level slot, forcing the required peer chain to nest, but now the required chain wins and the optional subtree nests instead.

both before/after were valid trees (two copies of the conflicted peer, all edges valid, etc). I've added assertions for this, but it's worth noting that installs with conflicting optional peers can produce a differently shaped node_modules than before.

(ht to @pi0, who spotted this)

References

Fixes#9876

[jakebailey]

I also saw the Bluesky post and independently found the same fix. This is a lot more meaningful now that TypeScript has optional peer deps for platform specific binaries.

[martinrrm]

Thanks for addressing the comments, LGTM!

[danielroe]

not at all! thank you for the very thorough and helpful review. ❤️

[github-actions]

🎉 Backport to release/v11 created: #9903

[danielroe]

this is a PR aimed to skip fetching packuments for optional peers that won't be installed. this can be extremely costly.

for example, with an empty cache, npm install vite into an empty project installs 10 packages but fetches full packuments for playwright (16.6MB), @types/node (11MB), webdriverio, jsdom, sass, less, terser, and (via vite's optional peer on vitest) all of vitest's optional peers too:

	before	after
registry requests	86	54
cached registry data	~121MB	~72MB

loadPeerSet resolves unmet optional peer edges into the virtual root, even though they won't be used and in fact be pruned later on

the fix is very simple; we just skip peerOptional edges if they have no current resolution and no parent edge. if something else ends up installing the package, the edge is still validated at placement time.

having said all of that, I'm a relative newcomer to the npm cli, so do let me know if there's a reason for this fetching that I'm missing 🙏

Note

the nest when peerOptional conflicts snapshot did change shape.

imo it was a bug before - the unmet optional peer was eagerly placed and 'won' the top-level slot, forcing the required peer chain to nest, but now the required chain wins and the optional subtree nests instead.

both before/after were valid trees (two copies of the conflicted peer, all edges valid, etc). I've added assertions for this, but it's worth noting that installs with conflicting optional peers can produce a differently shaped node_modules than before.

(ht to @pi0, who spotted this)

References

Fixes#9876

[jakebailey]

I also saw the Bluesky post and independently found the same fix. This is a lot more meaningful now that TypeScript has optional peer deps for platform specific binaries.

[martinrrm]

Thanks for addressing the comments, LGTM!

[danielroe]

not at all! thank you for the very thorough and helpful review. ❤️

[github-actions]

🎉 Backport to release/v11 created: #9903

[danielroe]

this is a PR aimed to skip fetching packuments for optional peers that won't be installed. this can be extremely costly.

for example, with an empty cache, npm install vite into an empty project installs 10 packages but fetches full packuments for playwright (16.6MB), @types/node (11MB), webdriverio, jsdom, sass, less, terser, and (via vite's optional peer on vitest) all of vitest's optional peers too:

	before	after
registry requests	86	54
cached registry data	~121MB	~72MB

loadPeerSet resolves unmet optional peer edges into the virtual root, even though they won't be used and in fact be pruned later on

the fix is very simple; we just skip peerOptional edges if they have no current resolution and no parent edge. if something else ends up installing the package, the edge is still validated at placement time.

having said all of that, I'm a relative newcomer to the npm cli, so do let me know if there's a reason for this fetching that I'm missing 🙏

Note

the nest when peerOptional conflicts snapshot did change shape.

imo it was a bug before - the unmet optional peer was eagerly placed and 'won' the top-level slot, forcing the required peer chain to nest, but now the required chain wins and the optional subtree nests instead.

both before/after were valid trees (two copies of the conflicted peer, all edges valid, etc). I've added assertions for this, but it's worth noting that installs with conflicting optional peers can produce a differently shaped node_modules than before.

(ht to @pi0, who spotted this)

References

Fixes#9876

[jakebailey]

I also saw the Bluesky post and independently found the same fix. This is a lot more meaningful now that TypeScript has optional peer deps for platform specific binaries.

[martinrrm]

Thanks for addressing the comments, LGTM!

[danielroe]

not at all! thank you for the very thorough and helpful review. ❤️

[github-actions]

🎉 Backport to release/v11 created: #9903

[danielroe]

this is a PR aimed to skip fetching packuments for optional peers that won't be installed. this can be extremely costly.

for example, with an empty cache, npm install vite into an empty project installs 10 packages but fetches full packuments for playwright (16.6MB), @types/node (11MB), webdriverio, jsdom, sass, less, terser, and (via vite's optional peer on vitest) all of vitest's optional peers too:

	before	after
registry requests	86	54
cached registry data	~121MB	~72MB

loadPeerSet resolves unmet optional peer edges into the virtual root, even though they won't be used and in fact be pruned later on

the fix is very simple; we just skip peerOptional edges if they have no current resolution and no parent edge. if something else ends up installing the package, the edge is still validated at placement time.

having said all of that, I'm a relative newcomer to the npm cli, so do let me know if there's a reason for this fetching that I'm missing 🙏

Note

the nest when peerOptional conflicts snapshot did change shape.

imo it was a bug before - the unmet optional peer was eagerly placed and 'won' the top-level slot, forcing the required peer chain to nest, but now the required chain wins and the optional subtree nests instead.

both before/after were valid trees (two copies of the conflicted peer, all edges valid, etc). I've added assertions for this, but it's worth noting that installs with conflicting optional peers can produce a differently shaped node_modules than before.

(ht to @pi0, who spotted this)

References

Fixes#9876

[jakebailey]

I also saw the Bluesky post and independently found the same fix. This is a lot more meaningful now that TypeScript has optional peer deps for platform specific binaries.

[martinrrm]

Thanks for addressing the comments, LGTM!

[danielroe]

not at all! thank you for the very thorough and helpful review. ❤️

[github-actions]

🎉 Backport to release/v11 created: #9903

[danielroe]

this is a PR aimed to skip fetching packuments for optional peers that won't be installed. this can be extremely costly.

for example, with an empty cache, npm install vite into an empty project installs 10 packages but fetches full packuments for playwright (16.6MB), @types/node (11MB), webdriverio, jsdom, sass, less, terser, and (via vite's optional peer on vitest) all of vitest's optional peers too:

	before	after
registry requests	86	54
cached registry data	~121MB	~72MB

loadPeerSet resolves unmet optional peer edges into the virtual root, even though they won't be used and in fact be pruned later on

the fix is very simple; we just skip peerOptional edges if they have no current resolution and no parent edge. if something else ends up installing the package, the edge is still validated at placement time.

having said all of that, I'm a relative newcomer to the npm cli, so do let me know if there's a reason for this fetching that I'm missing 🙏

Note

the nest when peerOptional conflicts snapshot did change shape.

imo it was a bug before - the unmet optional peer was eagerly placed and 'won' the top-level slot, forcing the required peer chain to nest, but now the required chain wins and the optional subtree nests instead.

both before/after were valid trees (two copies of the conflicted peer, all edges valid, etc). I've added assertions for this, but it's worth noting that installs with conflicting optional peers can produce a differently shaped node_modules than before.

(ht to @pi0, who spotted this)

References

Fixes#9876

[jakebailey]

I also saw the Bluesky post and independently found the same fix. This is a lot more meaningful now that TypeScript has optional peer deps for platform specific binaries.

[martinrrm]

Thanks for addressing the comments, LGTM!

[danielroe]

not at all! thank you for the very thorough and helpful review. ❤️

[github-actions]

🎉 Backport to release/v11 created: #9903

[danielroe]

this is a PR aimed to skip fetching packuments for optional peers that won't be installed. this can be extremely costly.

for example, with an empty cache, npm install vite into an empty project installs 10 packages but fetches full packuments for playwright (16.6MB), @types/node (11MB), webdriverio, jsdom, sass, less, terser, and (via vite's optional peer on vitest) all of vitest's optional peers too:

	before	after
registry requests	86	54
cached registry data	~121MB	~72MB

loadPeerSet resolves unmet optional peer edges into the virtual root, even though they won't be used and in fact be pruned later on

the fix is very simple; we just skip peerOptional edges if they have no current resolution and no parent edge. if something else ends up installing the package, the edge is still validated at placement time.

having said all of that, I'm a relative newcomer to the npm cli, so do let me know if there's a reason for this fetching that I'm missing 🙏

Note

the nest when peerOptional conflicts snapshot did change shape.

imo it was a bug before - the unmet optional peer was eagerly placed and 'won' the top-level slot, forcing the required peer chain to nest, but now the required chain wins and the optional subtree nests instead.

both before/after were valid trees (two copies of the conflicted peer, all edges valid, etc). I've added assertions for this, but it's worth noting that installs with conflicting optional peers can produce a differently shaped node_modules than before.

(ht to @pi0, who spotted this)

References

Fixes#9876

[jakebailey]

I also saw the Bluesky post and independently found the same fix. This is a lot more meaningful now that TypeScript has optional peer deps for platform specific binaries.

[martinrrm]

Thanks for addressing the comments, LGTM!

[danielroe]

not at all! thank you for the very thorough and helpful review. ❤️

[github-actions]

🎉 Backport to release/v11 created: #9903

[danielroe]

this is a PR aimed to skip fetching packuments for optional peers that won't be installed. this can be extremely costly.

for example, with an empty cache, npm install vite into an empty project installs 10 packages but fetches full packuments for playwright (16.6MB), @types/node (11MB), webdriverio, jsdom, sass, less, terser, and (via vite's optional peer on vitest) all of vitest's optional peers too:

	before	after
registry requests	86	54
cached registry data	~121MB	~72MB

loadPeerSet resolves unmet optional peer edges into the virtual root, even though they won't be used and in fact be pruned later on

the fix is very simple; we just skip peerOptional edges if they have no current resolution and no parent edge. if something else ends up installing the package, the edge is still validated at placement time.

having said all of that, I'm a relative newcomer to the npm cli, so do let me know if there's a reason for this fetching that I'm missing 🙏

Note

the nest when peerOptional conflicts snapshot did change shape.

imo it was a bug before - the unmet optional peer was eagerly placed and 'won' the top-level slot, forcing the required peer chain to nest, but now the required chain wins and the optional subtree nests instead.

both before/after were valid trees (two copies of the conflicted peer, all edges valid, etc). I've added assertions for this, but it's worth noting that installs with conflicting optional peers can produce a differently shaped node_modules than before.

(ht to @pi0, who spotted this)

References

Fixes#9876

[jakebailey]

I also saw the Bluesky post and independently found the same fix. This is a lot more meaningful now that TypeScript has optional peer deps for platform specific binaries.

[martinrrm]

Thanks for addressing the comments, LGTM!

[danielroe]

not at all! thank you for the very thorough and helpful review. ❤️

[github-actions]

🎉 Backport to release/v11 created: #9903