fix: make omit flags work properly with workspaces

[Rayyan98]

I am encountering an issue in using work-spaces whereby if i have a dev dependency at the root of my project with some version v1 and the same dependency inside one of the work-spaces which is a sub-folder in the project root with version v2 also as dev dependency that conflicts with v1 and try to do npm ci --omit=dev it skips installing the v1 dependency which was at project root but still installs the v2 dependency that was in the child workspace. I would expect that v2 should also not be installed. I have raised an issue for it here #6441 which also includes a link to minimum reproducible example.

In reify under arborist I found a filter condition which runs through the idealTree inventory to see which packages needed to be filtered out when omit dev or omit peer options etc are passed. Along with checking which flag is enabled and which packages is which type it also includes a condition of node.top.isProjectRoot. I don't fully understand the purpose of the last condition but am guessing, it is there so that the package is only filtered out if it is going to be directly under the root node_modules folder and not if it is going to be under the node_modules of some package which is under the actual root node_modules folder. I have extended the condition so that it is also filtered out if its top is a workspace which i think would translate to, that it should also be filtered out if its directly under the node_modules of a workspace folder.

References

Fixes #6441

PS: It is my first time contributing to open source please excuse me if i have not followed proper procedure or otherwise missed something,

[Rayyan98]

Adding screen shot for better clarification.

On running npm i --omit=dev, the package1/node_modules/dotenv dev dependency is installed anyway

[Rayyan98]

Changed commit message to make it inline with lint rules

[Rayyan98]

Updated PR Title to comply with lint rules

[Rayyan98]

Updated PR Title to comply with lint rules (2)

[wraithgar]

Is there a test that we can have to show this behavior working as intended?

[wraithgar]

Don't worry too much about linting the commit, we purposely made our CI look at the PR or the commit(s) so that we could change it in the PR before landing it. It's not something we are asking contributors to fret about.

[Rayyan98]

Added a test case

[Rayyan98]

@wraithgar Do we have some ETA for this, we are waiting to shift our monorepo to workspaces

[lukekarrys]

@Rayyan98 Thank you for your patience with this. One thing I was wondering is how we would want this to behave is using --omit=dev in conjunction with the --workspace, --workspaces, or --include-workspace-root flags.

I pushed a commit to this PR where it will filter the omit list based on those flags. Your originally test case still passed, and I added a few more. Would you be able to confirm if shifting your monorepo to workspaces still works as expected with this additional change?

[Rayyan98]

Yes if the original test case is passing then it should work fine

[lukekarrys]

@wraithgar This should be squashed and merged

[wraithgar]

Thank you @Rayyan98

[spearmootz]

i think this still happens
if i install @feathersjs/configuration i end up with something like this

├─┬ @feathersjs/configuration@5.0.25
│ └─┬ @feathersjs/schema@5.0.25
│ └── typescript@5.4.5 deduped

but they are all dev-dependencies and yet when i install with --omit dev or --omit=dev i always get typscript installed again