iframe-proxy

G-Rath · 2025-07-10T20:53:47Z

Currently optional peer dependencies are retained so long as at least one package in the tree has them as an optional peer dependency, meaning once added they become non-optional even though npm does actually seem to mark such dependencies.

To resolve this, I've modified the pruning logic to check for nodes that are both optional and peer, in addition to extraneous nodes.

References

Resolves #4737

G-Rath · 2025-07-10T21:35:48Z

wraithgar · 2025-07-10T22:11:41Z

I'm also not sure why this fails only on Windows

Yeah this is a GOOD thing cause answering this question will help us know if this PR is right.

G-Rath · 2025-07-11T02:44:16Z

@wraithgar so I've spent a few hours trying to dig into this without much luck, though I'm still pretty sure the failure is about the test setup rather than the fix being wrong - if I revert the fix, then all the tests fail with the same kind of error though there's more of them because it's resolving about 20 more packages.

I think the issue is something on the lines of dependencies (not) being cached - the error gets raised when calling this:

cli/workspaces/arborist/lib/arborist/reify.js

Lines 799 to 803 in c457c75

    
           await pacote.extract(res, node.path, { 
        
             ...this.options, 
        
             resolved: node.resolved, 
        
             integrity: node.integrity, 
        
           })

I also feel like when I revert the fix, the Windows test fail about different packages e.g. it complains about json-parse-even-better-errors whereas the Linux tests don't which could make sense if its related to caches and such because that package is a dependency used by NPM.

I don't think I'll be able to dig into this much further by myself I'm afraid as I've not found any solid difference between Windows and Linux 😞

G-Rath · 2025-07-16T22:27:18Z

I hadn't realized that node_modules/pacote is actually included in git, so that made it easy to patch in some logging and doing so shows this is because of caching:

What's interesting is that this patching also applies to the rest of the CI so I can see that dedent is being installed as part of the "reset deps" step which I assume is how it ends up in the cache resulting in the Linux and macOS CIs running...

G-Rath · 2025-07-17T00:06:57Z

ok so the issue is that the wrong cache directory is being used on Windows:

pacote looks in the home directory for the cache, but on Windows the cache is being stored in C:\npm

G-Rath · 2025-07-17T02:31:48Z

@wraithgar I've got the tests passing in a manner that you might not be too impressed by, but that I think at least shows the issue isn't with my change

wraithgar · 2025-07-17T15:33:25Z

The workaround is very intentional and well commented. It's always these one-line tweaks that get so far off the rails when it comes to testing. Thank you for seeing this through.

wraithgar · 2025-07-17T15:34:24Z

It's possible the mock npm fixture has a quirk in windows with this caching but fixing that is way outside the scope of this PR.

lkraav · 2025-07-21T12:56:56Z

Is there a chance of this getting backported?

wraithgar · 2025-07-21T15:09:32Z

@lkraav we typically don't actively do backports but this would probably be something we would consider if a PR was made against the v10 branch

G-Rath · 2025-07-21T20:40:14Z

I have opened #8449 backporting this, unfortunately CI for Linux and macOS is failing - since (ironically) Windows is passing, I'm guessing it's cache related and I wasn't able to figure out the root cause for this PR.

My very hacky work for exploring this is in https://github.com/G-Rath/cli/tree/prune-optional-peer2 if someone wants to have a crack at digging further - I'm not sure if I'll have the time this week

jenseng · 2025-08-28T15:39:17Z

Yeah, this change should be reverted or revisited. We tried upgrading our repos to npm 11.5.x, and this broke npm install in many of them, since esbuild has some postinstall logic to ensure that the correct versions of platform-specific optional deps are present. Since npm now incorrectly prunes them, that postinstall script fails. See #8536 for more details and a repro.

jenseng · 2025-08-28T16:22:29Z

A potential forward fix is hinted at by this comment, as well as #8489:

the tl;dr is that npm at least since latest v10 seems to use the "last" encounter with a particular dependency as the final resolution of whether it's optional/dev/peer/etc, so my particular optional (non-peer) dev dep is getting flagged as an optional peer dep because the "last" workspace in my setup happens to only depend on it as an optional peer.

I.e. it sounds like the ideal tree is incorrect, and so these deps end up getting pruned. If we can fix the ideal tree, then this PR may not actually be problematic.

silverwind · 2025-08-29T06:45:21Z

I think there is still some bug that causes unstable lockfile content for optional peer dependencies. I don't think it's related to platform-specific dependencies, seen it with any kind of optional peer dependencies.

liamcmitchell · 2025-08-29T10:48:36Z

so the Windows tests are failing because my new test results in a request for the dedent package and if I've understood all the other tests they don't result in a package after pruning so they avoid needing to do any mocking - I don't mind removing dedent from my test too if thats ok; otherwise I'll need guidance on how to address this as my attempts so far (mainly around mocking out the dependency / registry) have failed.

I've kept the adding of the tgz file in case that will be useful - I'm also not sure why this fails only on Windows 😕

Your workaround didn't work for me running tests locally. I avoided network by adding the dep to fixture node_modules in #8540

owlstronaut · 2025-09-16T22:26:27Z

@G-Rath my current leaning is to revert this change. I think it is correct, and a good change, but the side effects of other bugs have made this a pretty wide-reaching breaking change for folks. Since the flag fixes are pretty deep internals, I'm not confident we can get a fix for how peer/optional flags flow through the system working correctly quickly. I know that would be disappointing - do you have thoughts about that or alternatives?

G-Rath · 2025-09-17T02:23:31Z

@owlstronaut if you think that's best, then go for it - frankly, not meaning to be rude but what would be most disappointing about that is I don't expect it'll get prioritized anytime soon given the original issue (like some of my others) was open for 2+ years with seemingly next to no maintainer discussion or indication that it had been considered for trying to address 🙃

I would be interested in knowing if you're saying this primarily because of the feedback in this repo or if you've also received other feedback elsewhere (such as from enterprise folks).

wraithgar · 2025-09-19T00:59:24Z

- reverts pruning added in #8431, which incorrectly prunes deps flagged as `peer` and `optional` - these flags don't mean that this node is an optional peer! - reverts much of #8579, which I think mistakenly changed peer dep calculation logic - rewrites calcDepFlags - adds logic to avoid unsetting `extraneous` when following optional peer edges (how #8431, should have been fixed) - updates my prev fix to avoid looking for missing optional peer deps (`if ((!edge.to && edge.type !== 'peerOptional') || !edge.valid) {`) - refactors dep flag unsetting and resetting into Node methods - removes `shake out Link target timing issue` test, which was testing code [removed](2db6c08#diff-6778dbd4bbfddaeb827a8d2aa7248d4c9b329229f69e407d5fd487abe16dd942L333) a while back - avoids omitting flaky`selflink` fixture when writing snapshots Fixes #8535

… breaking change in npm 11.5.0 in how it handles optional peer dependencies. See the discussion in npm/cli#8431 for more details.

This PR: - Adds an ignore rule to cspell for GHSA IDs. - Bumps node to v24 for the repo (which is bundled with npm v11), updating the lint CircleCI jobs and release GitHub action as well. - This is because npm trusted publishing requires at least npm v11.5.1. - Stops passing `NPM_TOKEN` to `changesets/action` in the release GitHub action. - Note that npm versions that support trusted publishing will [ignore locally configured tokens when OIDC environment variables are present](https://github.com/npm/cli/blob/8afa3bd21461c0984caf1bcc2e486c4881bda516/lib/utils/oidc.js#L141) (which they are for GH actions, since we've set that up), so `NPM_TOKEN` would be ignored anyway. But the main reason for this change is to stop `changesets/action` from printing a misleading log message saying `NPM_TOKEN` has been placed into `.npmrc` (which, while true, will be ignored by `npm publish`). - Note that we still need to keep the `NPM_TOKEN` GitHub secret around to run any tag-changing scripts, as `npm dist-tag` sadly doesn't support OIDC yet (see npm/cli#8547 for discussion). This means we'll need to manually rotate the token every 90 days. - Pins the transitive dev dependency `@inquirer/external-editor` to `1.0.0`. - The gist is that npm v11.5.0 introduced a breaking change in how unused optional peer dependencies are handed with npm/cli#8431 (see the discussion there for details). Specifically, the change causes those dependencies to be removed from `package-lock.json`, and this will cause `npm ci` in earlier versions of npm to error, complaining about `package-lock.json` being out of sync with `package.json`. `@inquirer/external-editor` introduced such an optional peer dependency in `1.0.1`, and the simplest fix here is to just fallback to `1.0.0`. - Uses `npm ci` instead of `npm i` in the release GitHub action. - As a requirement for releasing (or filing a release PR), the `package-lock.json` should be aligned with the `package.json`. - Stops setting `FEDERATION_VERSION` in the release GitHub action since it's no longer used. - Sets the tag for npm publishes to the branch name for `version-*` branches. - Previously, it was the default of `latest`, which was causing backport releases to mistakenly change the `latest` tag (used when someone `npm i`s without a version number). - We also tightened the `version-*` branch pattern down to `version-[0-9]+.[0-9]+` (since it's getting passed around in bash now).

…lique allowScripts Depuis que Dependabot a régénéré package-lock.json avec npm 11 (#118, #119), npm ci échoue avec npm 10 (Node 22) et avec npm 11 avant 11.5.0 : EUSAGE, « Missing: chokidar@5.0.0 / readdirp@5.1.1 from lock file ». npm 11.5.0 ne garde plus dans le lock les dépendances peer optionnelles que rien ne demande (npm/cli#8431). Or engines n'acceptait que Node 22, 24 et 26, quel que soit leur npm. - engines.npm >=11.18.0 : avec engine-strict, un npm plus ancien refuse l'installation (EBADENGINE), en donnant la version exigée. 11.5.0 suffit pour lire le lock, mais npm ignore strict-allow-scripts (#123) avant 11.16.0, et 11.16 et 11.17 rejettent fsevents (optionnel, macOS seulement) même là où il n'est pas installé (npm/cli#9602). - engines.node ne change pas : Node 22 (npm 10), Node 24 avant 24.20 et Node 26 avant 26.6 passent avec un npm 11 récent installé à part. - Le lock recopie engines. - CLAUDE.md dit pourquoi npm 11.18.0 est requis. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

G-Rath added 3 commits July 10, 2025 20:48

test: add fixture

7f02c11

fix: prune optional peer dependencies from tree

2b0f8e5

test: mock the audit endpoint

f6aa3dd

G-Rath marked this pull request as ready for review July 10, 2025 21:04

G-Rath requested a review from a team as a code owner July 10, 2025 21:04

G-Rath mentioned this pull request Jul 10, 2025

[BUG] uninstalling an optional peer dep doesnt remove it #4737

Closed

2 tasks done

G-Rath added 2 commits July 10, 2025 21:18

test: attempt to get Windows passing

7964883

test: rollback

efd74f7

wraithgar reviewed Jul 10, 2025

View reviewed changes

Comment thread workspaces/arborist/lib/arborist/build-ideal-tree.js

chore: add comment

00df4af

wraithgar self-assigned this Jul 10, 2025

G-Rath added 2 commits July 17, 2025 12:39

test: explicitly set the cache directory on Windows

e028d60

chore: address linting errors

9db43c0

G-Rath requested a review from wraithgar July 17, 2025 02:31

wraithgar approved these changes Jul 17, 2025

View reviewed changes

wraithgar merged commit 8042af3 into npm:latest Jul 17, 2025

github-actions Bot mentioned this pull request Jul 17, 2025

chore: release 11.5.0 #8377

Merged

G-Rath deleted the prune-optional-peer branch July 17, 2025 18:58

G-Rath mentioned this pull request Jul 21, 2025

fix: prune optional peer dependencies that are no longer explicitly depended on #8449

Closed

billyjanitsch mentioned this pull request Aug 5, 2025

[BUG] npm fails to install optional dependencies of peer dependencies #8487

Closed

2 tasks done

acburdine mentioned this pull request Aug 6, 2025

[BUG] npm workspace incorrectly flags dependency as optional peer #8489

Closed

2 tasks done

This was referenced Aug 17, 2025

[BUG] Cannot find module '../lightningcss.linux-x64-gnu.node' #8516

Closed

Cannot find module '../lightningcss.linux-x64-gnu.node' parcel-bundler/lightningcss#1041

Open

jenseng mentioned this pull request Aug 27, 2025

[BUG] npm install incorrectly prunes unhoisted transitive optional dependencies when using workspaces #8536

Closed

2 tasks done

louis-bompart mentioned this pull request Sep 17, 2025

chore: sync-up arborist & npm, add missing vitest.config.js, add missing eslint-parser coveo/ui-kit#6048

Merged

owlstronaut mentioned this pull request Sep 17, 2025

fix: corrects peer dependency flag propagation #8579

Merged

dynamictulip mentioned this pull request Sep 28, 2025

[BUG] npm install creates unsync package-lock.json #6787

Closed

2 tasks done

liamcmitchell mentioned this pull request Oct 5, 2025

fix: dep flag calculation #8645

Merged

mgermerie mentioned this pull request Nov 4, 2025

[Chores] Update node supported version to 24 iTowns/itowns#2628

Merged

liamcmitchell mentioned this pull request Nov 26, 2025

[BUG] On v11.6.1 npm i or npm update generates different lock file contents on second run #8725

Closed

2 tasks done

lukasrad02 mentioned this pull request Dec 2, 2025

Update Angular to fix security issues hpi-sam/fuesim-digital#1180

Merged

4 tasks done

liamcmitchell mentioned this pull request Dec 8, 2025

[BUG] npm install produces different results on first and second run #8535

Closed

2 tasks done

kroney mentioned this pull request Jan 14, 2026

npm 11.7.0 and current nodejs24 npm 11.6.2 are incompatible dependabot/dependabot-core#13931

Closed

1 task done

sachindshinde mentioned this pull request Mar 14, 2026

chore: Use node 24 and npm trusted publishing, and update cspell apollographql/federation#3411

Merged

trentm mentioned this pull request Jun 4, 2026

chore(deps): update @arethetypeswrong/core to 0.18.3 open-telemetry/opentelemetry-browser#300

Merged

3 of 5 tasks

shahirtaj mentioned this pull request Jul 12, 2026

ci: Build on Node 24 and guard @next/swc libc fields shahirtaj/doublecheck#28

Merged

andrew-ahlfield mentioned this pull request Jul 16, 2026

Put the CI/toolchain problem to bed: pin Node, block the bad npm window, fix the cloud test gap andrew-ahlfield/emfe#13

Merged

yann-disanto mentioned this pull request Oct 16, 2026

chore(engines): exiger npm 11.18.0, le premier qui lit le lock et applique allowScripts recos-diet/recos-diet#129

Merged

Sunbelt Computer Software

PL/B Language Development and Support

Conversation

G-Rath commented Jul 10, 2025

References

Uh oh!

G-Rath commented Jul 10, 2025

Uh oh!

Uh oh!

wraithgar commented Jul 10, 2025

Uh oh!

G-Rath commented Jul 11, 2025

Uh oh!

G-Rath commented Jul 16, 2025

Uh oh!

G-Rath commented Jul 17, 2025

Uh oh!

G-Rath commented Jul 17, 2025

Uh oh!

wraithgar commented Jul 17, 2025

Uh oh!

wraithgar commented Jul 17, 2025

Uh oh!

lkraav commented Jul 21, 2025

Uh oh!

wraithgar commented Jul 21, 2025

Uh oh!

G-Rath commented Jul 21, 2025

Uh oh!

jenseng commented Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jenseng commented Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

silverwind commented Aug 29, 2025

Uh oh!

liamcmitchell commented Aug 29, 2025

Uh oh!

owlstronaut commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

G-Rath commented Sep 17, 2025

Uh oh!

wraithgar commented Sep 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

jenseng commented Aug 28, 2025 •

edited

Loading

jenseng commented Aug 28, 2025 •

edited

Loading

owlstronaut commented Sep 16, 2025 •

edited

Loading