Berry: [Bug?] Virtual paths cause duplicate module instantiation

Created on 8 Mar 2020  ยท  15Comments  ยท  Source: yarnpkg/berry

  • [ ] I'd be willing to implement a fix

Describe the bug

For Parcel , we have multiple modules that do this:

import {register} from "@parcel/core";
class X {}
register("X", X);

When installing Parcel via PnP, it seems like these modules are called multiple times (and register mustn't be called multiple times - this works fine with Yarn 1).

I found out that the problematic file (@parcel/package-manager/lib/Npm.js) is required & compiled via two different require chains, @parcel-package-manager-virtual-<TWO-DIFFERENT-IDS-HERE> (see below).

I known this is problematic and somewhat relies on hoisting (and I already get some peerdependency warnings).

To Reproduce

๐Ÿ˜ฌ

Example

Call chain 1

Trace: 2.0.0-alpha.3.2-dccf3d96:Npm
    at registerSerializableClass (pnp/.yarn/unplugged/@parcel-core-npm-2.0.0-alpha.3.2-dccf3d96-27a2fe7d38/node_modules/@parcel/core/lib/serializer.js:27:13)
    at Object.<anonymous> (pnp/.yarn/$$virtual/@parcel-package-manager-virtual-e6cacad87d/0/cache/@parcel-package-manager-npm-2.0.0-alpha.3.2-dccf3d96-3ccef4b8cf-2.zip/node_modules/@parcel/package-manager/lib/Npm.js:84:37)
    at Module._compile (internal/modules/cjs/loader.js:1158:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1178:10)
    at Module.load (internal/modules/cjs/loader.js:1002:32)
    at Function.module_1.Module._load (pnp/.pnp.js:23984:14)
    at Module.require (internal/modules/cjs/loader.js:1044:19)
    at require (internal/modules/cjs/helpers.js:77:18)
    at Object.<anonymous> (pnp/.yarn/$$virtual/@parcel-package-manager-virtual-e6cacad87d/0/cache/@parcel-package-manager-npm-2.0.0-alpha.3.2-dccf3d96-3ccef4b8cf-2.zip/node_modules/@parcel/package-manager/lib/index.js:19:12)
    at Module._compile (internal/modules/cjs/loader.js:1158:30)

Call chain 2

Trace: 2.0.0-alpha.3.2-dccf3d96:Npm
    at registerSerializableClass (pnp/.yarn/unplugged/@parcel-core-npm-2.0.0-alpha.3.2-dccf3d96-27a2fe7d38/node_modules/@parcel/core/lib/serializer.js:27:13)
    at Object.<anonymous> (pnp/.yarn/$$virtual/@parcel-package-manager-virtual-939cfbf528/0/cache/@parcel-package-manager-npm-2.0.0-alpha.3.2-dccf3d96-3ccef4b8cf-2.zip/node_modules/@parcel/package-manager/lib/Npm.js:84:37)
    at Module._compile (internal/modules/cjs/loader.js:1158:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1178:10)
    at Module.load (internal/modules/cjs/loader.js:1002:32)
    at Function.module_1.Module._load (pnp/.pnp.js:23984:14)
    at Module.require (internal/modules/cjs/loader.js:1044:19)
    at require (internal/modules/cjs/helpers.js:77:18)
    at Object.<anonymous> (pnp/.yarn/$$virtual/@parcel-package-manager-virtual-939cfbf528/0/cache/@parcel-package-manager-npm-2.0.0-alpha.3.2-dccf3d96-3ccef4b8cf-2.zip/node_modules/@parcel/package-manager/lib/index.js:19:12)
    at Module._compile (internal/modules/cjs/loader.js:1158:30)

Environment if relevant (please complete the following information):

  • OS: macOS
  • Node version: 12
  • Yarn version: 2.0.0-rc.29

Additional context

Add any other context about the problem here.

bug

Most helpful comment

but @parcel/package-manager is still duplicated...

Yep, I still need to look into it, there's a missing deduplication (cf my previous comment). Probably fixable on our side, will report back once I have more info ๐Ÿ‘

All 15 comments

This will only happen if the two instances have different resolved peer dependencies depending on their branch. For example, assuming the following, X will not be duplicated:

. -> A -> X ~> Y
       -> Y@1
  -> B -> X ~> Y
       -> Y@1

However the following tree will lead to X being duplicated, because the Y instance it must use isn't the same for both branches:

. -> A -> X ~> Y
       -> Y@1
  -> B -> X ~> Y
       -> Y@2

I think that's the expected behavior - does that make sense?

That does look correct. But this is essentially hash(resolved peerdeps), so the X could be unafftected. (In your example: X doesn't use Y).

Does that mean that this bottom-up register functionality is now impossible?

No, in my example X is the one which have a peer dependency on Y. The root depends on A and B, and both A and B depend on X and Y. For simplicity I assumed that X cannot be hoisted to the root (for example the root could have an incompatible X dependency itself).

From my poking around in.pnp.js, I think this is the situation:

parcel
    | - @parcel/core
    |   | - @parcel/fs$1 (has peerDep on @parcel/core)
    |   | - @parcel/package-manager$1 (has peerDep on @parcel/core)
    |   | - @parcel/source-map$1 (has peerDep on @parcel/core)
    | - @parcel/package-manager$2 (has peerDep on @parcel/core)
    | - @parcel/fs$2 (has peerDep on @parcel/core)
    | - @parcel/config-default (has peerDep on @parcel/core)
        | - @parcel/transformer-js (has peerDep on @parcel/core)
            | - @parcel/source-map$2 (has peerDep on @parcel/core)
        | - @parcel/transformer-sass (has peerDep on @parcel/core)
            | - @parcel/fs$2 (has peerDep on @parcel/core)

So it looks like this split occurs between (has peerdep and child of @parcel/core) and (has peerdep and is essentially a sibling). But I have no idea how to solve this...

@mischnic So the problem is with two different instances of @parcel/package-manager according to your stack trace. To troubleshoot this problem, we should look into the output of yarn why and determine the parents who use @parcel/package-manager, then we should look into dependencies of these parents and peerDependencies of @parcel/package-manager. This way we will know why Yarn has created two instances of @parcel/package-manager.

It seems the problem comes from workers, which doesn't seem properly deduplicated. I'll take a closer look to understand why there are two instances:

["@parcel/workers", [
    ["virtual:6308ed7a3b136a878ef15966aad820ea7565944722dbd99be72a115154d72c40952d60f9e7d743c487dd425078d6d430ed4a4c9940271e91856b1ec99571d3c9#npm:2.0.0-nightly.144", {
        "packageLocation": "./.yarn/$$virtual/@parcel-workers-virtual-be20c1a8d6/0/cache/@parcel-workers-npm-2.0.0-nightly.144-e3f417a2a4-2.zip/node_modules/@parcel/workers/",
        "packageDependencies": [
            ["@parcel/workers", "virtual:6308ed7a3b136a878ef15966aad820ea7565944722dbd99be72a115154d72c40952d60f9e7d743c487dd425078d6d430ed4a4c9940271e91856b1ec99571d3c9#npm:2.0.0-nightly.144"],
            ["@parcel/core", "npm:2.0.0-nightly.142"],
            ["@parcel/diagnostic", "npm:2.0.0-nightly.144"],
            ["@parcel/logger", "npm:2.0.0-nightly.144"],
            ["@parcel/utils", "npm:2.0.0-nightly.144"],
            ["chrome-trace-event", "npm:1.0.2"],
            ["nullthrows", "npm:1.1.1"]
        ],
        "packagePeers": [
            "@parcel/core"
        ],
        "linkType": "HARD"
    }],
    ["virtual:c512c1833f04bfcdb8b3d33e40a9f0c83701b873184eb85b9d6f6eab99c07cec006bf0194b0b6f65ea7612cf8c58da981caddce5e06e8e665d18a5e8f2889ab7#npm:2.0.0-nightly.144", {
        "packageLocation": "./.yarn/$$virtual/@parcel-workers-virtual-b75054fbbf/0/cache/@parcel-workers-npm-2.0.0-nightly.144-e3f417a2a4-2.zip/node_modules/@parcel/workers/",
        "packageDependencies": [
            ["@parcel/workers", "virtual:c512c1833f04bfcdb8b3d33e40a9f0c83701b873184eb85b9d6f6eab99c07cec006bf0194b0b6f65ea7612cf8c58da981caddce5e06e8e665d18a5e8f2889ab7#npm:2.0.0-nightly.144"],
            ["@parcel/core", "npm:2.0.0-nightly.142"],
            ["@parcel/diagnostic", "npm:2.0.0-nightly.144"],
            ["@parcel/logger", "npm:2.0.0-nightly.144"],
            ["@parcel/utils", "npm:2.0.0-nightly.144"],
            ["chrome-trace-event", "npm:1.0.2"],
            ["nullthrows", "npm:1.1.1"]
        ],
        "packagePeers": [
            "@parcel/core"
        ],
        "linkType": "HARD"
    }]
]]

Another issue is that various packages aren't passing @parcel/core to their peer dependencies, so it causes them to be duplicated (one instance has access to @parcel/core, while the other doesn't).

Thank you for taking a look.

Another issue is that various packages aren't passing @parcel/core to their peer dependencies, so it causes them to be duplicated (one instance has access to @parcel/core, while the other doesn't).

I'm on the pnp branch, publishing locally to verdaccio

then we should look into dependencies of these parents and peerDependencies of @parcel/package-manager. This way we will know why Yarn has created two instances of @parcel/package-manager.

niklas@nmb:pnp $ yarn why @parcel/package-manager
โ”œโ”€ @parcel/core@npm:2.0.0-alpha.3.2-c830673c
โ”‚  โ””โ”€ @parcel/package-manager@npm:2.0.0-alpha.3.2-c830673c [65275] (via npm:^2.0.0-alpha.3.2-c830673c [65275])
โ”‚
โ””โ”€ parcel@npm:2.0.0-alpha.3.2-c830673c
   โ””โ”€ @parcel/package-manager@npm:2.0.0-alpha.3.2-c830673c [57cf5] (via npm:^2.0.0-alpha.3.2-c830673c [57cf5])

What exactly are the conditions for that? (and what does [65275] mean?)

Another issue is that various packages aren't passing @parcel/core to their peer dependencies, so it causes them to be duplicated (one instance has access to @parcel/core, while the other doesn't).

Maybe we should unite such duplicates, because all bets are already off, then what is the reason to keep two duplicates?

EDIT: We cannot in case of PnP because, we should not allow access to @parcel/core if it was not provided by the parent...

so it causes them to be duplicated (one instance has access to @parcel/core, while the other doesn't).

I have fixed all of these (and I get no warning anymore at install time), but @parcel/package-manager is still duplicated...

but @parcel/package-manager is still duplicated...

Yep, I still need to look into it, there's a missing deduplication (cf my previous comment). Probably fixable on our side, will report back once I have more info ๐Ÿ‘

We encountered the same problem with the react component library of nivo.

โžœ yarn why @nivo/axes
โ”œโ”€ @nivo/bar@npm:0.61.1
โ”‚  โ””โ”€ @nivo/axes@npm:0.61.0 (via npm:0.61.0)
โ”‚
โ”œโ”€ @nivo/bar@npm:0.61.1 [f7a75]
โ”‚  โ””โ”€ @nivo/axes@npm:0.61.0 [cfaf4] (via npm:0.61.0 [cfaf4])
โ”‚
โ”œโ”€ @nivo/line@npm:0.61.1
โ”‚  โ””โ”€ @nivo/axes@npm:0.61.0 (via npm:0.61.0)
โ”‚
โ”œโ”€ @nivo/line@npm:0.61.1 [f7a75]
โ”‚  โ””โ”€ @nivo/axes@npm:0.61.0 [8b627] (via npm:0.61.0 [8b627])
โ”‚
โ”œโ”€ @nivo/scatterplot@npm:0.61.1
โ”‚  โ””โ”€ @nivo/axes@npm:0.61.0 (via npm:0.61.0)
โ”‚
โ””โ”€ @nivo/scatterplot@npm:0.61.1 [f7a75]
   โ””โ”€ @nivo/axes@npm:0.61.0 [7a6a8] (via npm:0.61.0 [7a6a8])

even though they have a locked-in version and have exactly the same peer-dependencies - multiple versions of the package (with different hashes) are created. therefore the react context is not working...

"@nivo/axes@npm:0.61.0":
  version: 0.61.0
  resolution: "@nivo/axes@npm:0.61.0"
  dependencies:
    "@nivo/core": 0.61.0
    d3-format: ^1.3.2
    d3-time: ^1.0.11
    d3-time-format: ^2.1.3
    lodash: ^4.17.11
    react-motion: ^0.5.2
  peerDependencies:
    prop-types: ">= 15.5.10 < 16.0.0"
    react: ">= 16.8.4 < 17.0.0"
  checksum: 2/f5c35d94b8326bcd537fb58e4de47bb16a7ea4d8b47b77e9e6fee2f85a7637f70617f5bc91d6c8387b5cf50fff65709e4d78ae89ff8cac9479b41c7656179ab1
  languageName: node
  linkType: hard

"@nivo/bar@npm:^0.61.0":
  version: 0.61.1
  resolution: "@nivo/bar@npm:0.61.1"
  dependencies:
    "@nivo/annotations": 0.61.0
    "@nivo/axes": 0.61.0
    "@nivo/colors": 0.61.0
    "@nivo/core": 0.61.0
    "@nivo/legends": 0.61.1
    "@nivo/tooltip": 0.61.0
    d3-scale: ^3.0.0
    d3-shape: ^1.2.2
    lodash: ^4.17.11
    react-motion: ^0.5.2
    recompose: ^0.30.0
  peerDependencies:
    prop-types: ">= 15.5.10 < 16.0.0"
    react: ">= 16.8.4 < 17.0.0"
  checksum: 2/b7b9721a173bef399ff31076e0adb2b42144f4b7aa80f234c42586a32142903780a3d99062ea58b7566bb9dc5cb4b69ea835275874304e955e4ad8e62b51e777
  languageName: node
  linkType: hard

@mischnic @ChiefORZ can you confirm that the following branch fixes the problem?: https://github.com/yarnpkg/berry/pull/1051

yarn set version from sources --branch 1051

Yes! ๐ŸŽ‰

yeah! ๐Ÿ˜Ž

Thanks for the fast response. I think the Parcel PnP PR is ready

Was this page helpful?
0 / 5 - 0 ratings

Related issues

tiansijie picture tiansijie  ยท  4Comments

milichev picture milichev  ยท  3Comments

kiprasmel picture kiprasmel  ยท  3Comments

danreg picture danreg  ยท  3Comments

mormahr picture mormahr  ยท  3Comments