From c43f66d14b1baedc7f50055dd7c997b32be0f8b1 Mon Sep 17 00:00:00 2001 From: Daniel Tschinder Date: Fri, 22 Nov 2019 12:26:47 +0100 Subject: [PATCH] Correctly install workspace child deps when workspace child not symlinked to root (#7289) * Test for conflict between pkg versions in root and workspace child This test creates the scenario where the child is not symlinked into the root node_modules folder, because the root installs a different version of the workspace child from the registry. * fix(hoister): Correctly change path for ws child dependencies when child not hoisted * Update CHANGELOG.md * Better comment --- CHANGELOG.md | 4 +++ .../commands/install/workspaces-install.js | 19 ++++++++++ .../arrify/package.json | 7 ++++ .../package.json | 11 ++++++ .../arrify/-/arrify-1.0.0.tgz.bin | Bin 0 -> 2064 bytes .../isarray/-/isarray-2.0.0.tgz.bin | Bin 0 -> 2643 bytes src/package-hoister.js | 33 ++++++++++++++---- src/package-linker.js | 4 --- 8 files changed, 67 insertions(+), 11 deletions(-) create mode 100644 __tests__/fixtures/install/workspaces-install-conflict-without-symlink/arrify/package.json create mode 100644 __tests__/fixtures/install/workspaces-install-conflict-without-symlink/package.json create mode 100644 __tests__/fixtures/request-cache/GET/registry.yarnpkg.com/arrify/-/arrify-1.0.0.tgz.bin create mode 100644 __tests__/fixtures/request-cache/GET/registry.yarnpkg.com/isarray/-/isarray-2.0.0.tgz.bin diff --git a/CHANGELOG.md b/CHANGELOG.md index d287c78698..e74725d637 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ Please add one entry in this file for each change in Yarn's behavior. Use the sa ## Master +- Correctly installs workspace child dependencies when workspace child not symlinked to root. + + [#7289](https://github.com/yarnpkg/yarn/pull/7289) - [**Daniel Tschinder**](https://github.com/danez) + - Makes running scripts with Plug'n Play possible on node 13 [#7650](https://github.com/yarnpkg/yarn/pull/7650) - [**Sander Verweij**](https://github.com/sverweij) diff --git a/__tests__/commands/install/workspaces-install.js b/__tests__/commands/install/workspaces-install.js index a0e0c0be15..5181352fc3 100644 --- a/__tests__/commands/install/workspaces-install.js +++ b/__tests__/commands/install/workspaces-install.js @@ -103,6 +103,25 @@ test.concurrent('install should install unhoistable dependencies in workspace no }); }); +test.concurrent( + 'install should install unhoistable dependencies in workspace node_modules even when no symlink exists', + (): Promise => { + return runInstall({}, 'workspaces-install-conflict-without-symlink', async (config): Promise => { + // node_modules/isarray@1.0.0 + let packageFile = await fs.readFile(path.join(config.cwd, 'node_modules', 'isarray', 'package.json')); + expect(JSON.parse(packageFile).version).toEqual('1.0.0'); + + // node_modules/arrify@1.0.0 + packageFile = await fs.readFile(path.join(config.cwd, 'node_modules', 'arrify', 'package.json')); + expect(JSON.parse(packageFile).version).toEqual('1.0.0'); + + // node_modules/arrify/isarray@2.0.0 + packageFile = await fs.readFile(path.join(config.cwd, 'arrify', 'node_modules', 'isarray', 'package.json')); + expect(JSON.parse(packageFile).version).toEqual('2.0.0'); + }); + }, +); + test.concurrent('install should link workspaces that refer each other', (): Promise => { return runInstall({}, 'workspaces-install-link', async (config): Promise => { // packages/workspace-1/node_modules/left-pad - missing because it is hoisted to the root diff --git a/__tests__/fixtures/install/workspaces-install-conflict-without-symlink/arrify/package.json b/__tests__/fixtures/install/workspaces-install-conflict-without-symlink/arrify/package.json new file mode 100644 index 0000000000..6bd08c8c85 --- /dev/null +++ b/__tests__/fixtures/install/workspaces-install-conflict-without-symlink/arrify/package.json @@ -0,0 +1,7 @@ +{ + "name": "arrify", + "version": "2.0.0", + "dependencies": { + "isarray": "2.0.0" + } +} diff --git a/__tests__/fixtures/install/workspaces-install-conflict-without-symlink/package.json b/__tests__/fixtures/install/workspaces-install-conflict-without-symlink/package.json new file mode 100644 index 0000000000..01b65e9fa7 --- /dev/null +++ b/__tests__/fixtures/install/workspaces-install-conflict-without-symlink/package.json @@ -0,0 +1,11 @@ +{ + "name": "my-project", + "private": true, + "dependencies": { + "arrify": "1.0.0", + "isarray": "1.0.0" + }, + "workspaces": [ + "arrify" + ] +} diff --git a/__tests__/fixtures/request-cache/GET/registry.yarnpkg.com/arrify/-/arrify-1.0.0.tgz.bin b/__tests__/fixtures/request-cache/GET/registry.yarnpkg.com/arrify/-/arrify-1.0.0.tgz.bin new file mode 100644 index 0000000000000000000000000000000000000000..2f5b1d63b4d881f658f75069534308e711486cf2 GIT binary patch literal 2064 zcmYjPX;4#X7L6>5Ku`xIvS^+H+6vhhl1GA#kwqbmKtwjd$IHutki_J%L^c7}Alk^H zh`2GBLFs_n6bgulve^T)A}Z~IfG8!>Oeq7ckj?~qrkwB3S9R;0d(S!d`U?cxNfaUl zppwZzPyp6)3xpydz?aM1jU|8&sf{0n4UpL&nF3Nh06)F}Ysr(#P(+3j1Zss53@H=| zQW!#|av4buqX_HF(0ZNc2BUsB&BaT=TVUkqH6~a^@g(?!#MfA8hHiPEDV2BWy%78r}I*m#5fG7+Gi$d&ERo zp6Chqqo^WCmY_Du^(F8iSb`8jQ3y>^fq?(k`3Sz2=7Uhm6F?vlN+m#IgiEIxgCu_? zg+OW$pbObTDw)n;#!;zk8XYEtZsTDsygcf5Mhc$G&YS4vxP8otr3~3Fa}8A3BC_%AWhn>dw=nqWs(R@`=t<7m75N=;+}4s+{c6FPpLlExspI*oLKH z85_;;J@F-%`K3Q{dg-?ij|q_v|MW3)(&Kb(p}14KzqRPHt7)-mH>UsBTO~yu?uEXe zkK%UF?`ooz#d)Rc0?xVSbVVNv7zj_=UlM=TO8AhdW7#__u*g3Ewegu3IeNj%AZLEv zNc#2NZf)W~kzb!pj6SX+AYX`{R`UR^``1! zS}>(z(T9<^7rr4&_!?vIB7<$!dzA01P4(|V=Rjt_Q zK&A|Z1f6VgJyyE=DQ)HAGaE|6$;*##+m%@rAgr}H&_JQiFlTL~jt^KietYws18)@nY2qsj#D z*1GrOA!~x;FKun89Fe>+aR~^`aZr#=ZO6+~4*504j2*b5*8KhQi({X<^!_(pc-40# zC+$mJI*RP`CL^lpJ~x(pqIb(t35&We+Rr7{_LAJ}c8~U-_d|B&xbGZ`AIqYlH?WZ_6Y0RBI&xw8d zdc_fC27a>fFm6?`ua_iOwd0&)JhEQ)E8F)mdum?{Dl2d%z28&rmx-~lJ_~#VwcPdl z?@G!4;^Deef)fYF@#c~??%vsg{IG(Bzk6;0ILZ6Ci5ix6EaQSc_xL8(lBW8OWlpoV zb$R!BIIhYjO@>(JzMpcMyq}{}4L2p_7oCVSxiqkG^7V&j{Vs{UcVpLooNKnJ7@4WS z1Y1>DU#c7{#WmQmvT^SZIGO)>)CUEFTRpaI*G+~+qF07q9R>2*gLo-bOB-~3yj`gr zeOiaOrffEAm7PA8d?xTTckAkw_Q4E;RF`h>&iUOiQT{T?kY$~^u>Yyq%|C{QYu`vs z)-dZsVxDwtTk$ce;-Xo(@-y{qGSN4ml&-rricY1T6`9MYf8EYx4!k)>2#f{PR-(J953e-$+b-OGB<5QQ>E2Yg}>F z&B4J;=8Q$v>TyS;eI#0V=85D3zNPJ?@|H02Vn%-VoM+?*QsUEkRtjv)-xOht!W{-ehFon4?!+4A_X%FfK-gPik277tBd;_uvRyM5~? h(3W$uSNrp1#^R^7hZxlm_JmEA$!3S*ml!7u=09Dqb=Lp@ literal 0 HcmV?d00001 diff --git a/__tests__/fixtures/request-cache/GET/registry.yarnpkg.com/isarray/-/isarray-2.0.0.tgz.bin b/__tests__/fixtures/request-cache/GET/registry.yarnpkg.com/isarray/-/isarray-2.0.0.tgz.bin new file mode 100644 index 0000000000000000000000000000000000000000..55b2d8eec02ac7d67ef2770cc9ce772b29f9e60b GIT binary patch literal 2643 zcmYjP2{@E%8|F~5jHT%h(fAri3p30xV-{N(Wf@B`SyJ?w`HaajW6WTRBsp3{b|QPn zE=y5qEY)eWsMAW+$A=w0ER@yn38cMz}|%hQ=+iLcu*J*O%rgWWRSxNVbMVz ziydadrt=^kI)cZAz|dbjXDE!x3nH(!!u>?UAo?nu3A|aTPhqRKyW(TvR zJ^lUZfeb#2VZ|`T1rQlT1_Uy2Sjd!MMmNLZ2}Ha(BLHMz@pyvt63uX?By*Al07Y|H zTqwed#)nV$Ph{2!$F2rGTd1yYD zWrYlqdWayKnEb+w=^<=BBQONyLaR1S=!`Hloyjr@fIzzR_oVr)_NDWrGn~N)9@>S? zUQNYb;C& zrlfanoy^Z8H@RtrJeyNLCSTpgqH=5g{(&9{Jco<60a22GV| z?gz^r{`eYY@7R)Yz2}duf{7Zn-O!izRQoU8819>3=A-28E8|}2NmmNB46~eKnF^x= zmDAPi%h@d&)$@Bc`|S@tzlY!Ys`5q66~mX44)r8waW2 zg{5B4{JN0FE4k-LNC*9|f6Y3K=`J@SZ`a$&`#dQG2PGMd%C?vz2MywjzKM@|P*jjp zVq_RIx59c!SLza~dn9A*)G9Y-E3`kft-8vu-L)wz_dfCSotbCFX?m5!?KFh&at^iQ z{{F!4#S`(Aa_vj$K;P*&O!axU5!*P+GYO09hg{`TTKCmhy}!S=9!?HOOt5z*?%~5I zXBA&3Wm%6U>kjF(9i87I)FTz{6TR?D$oqVuPDCo%c70lS|2MjLlr$%Rgv`HZ78g5&9(9(vlC1GRe;m{J+mGUT zDyV^-#WdkA}__4f#P*PJ(z&>l0gMAjUcznp(dJICFc zajc--j%;PEEH@Qxq_i^TodYjgQ}`k_Y;bL0^mDw^=}7bZr>ao~S=p9jno5~oRjEM6 z_F<&omr|BsY<7%`QxX|WPR|tg_g)#Il^SR*FHB!4j~rRoO%=$otyPli2(ZrZ~IToWr{H76*)^N>EkDX(uzwJUY&k$a#E;8-p${R(wSlqd( zZH)1zvNxs~#f!4)DE6*br)4w-E8d*Lxw|XCH{Ql53Qttw&Td1wA#+Q9L#wsgTr`=l z%@mtzmwGSs7sJMT(yx~rWel)lt695IZrBS+mKT+);U%NlGC8TC*F6mGEbO{fqo50W^Sp(XpNA5 z6K&GvJY8S@Vp#EPTFizgx?LCgu>AttIT$oJ0V8NO?}s*eK-B! znW-Y@@A_bH@%N*-cl&l!c_g+uAJ-xlqz|1aleb*U(#qODu>qEc*tc^8Wmm2l8139f zp{5}Wkak(lGlwG`>v4Wq!k-AF-V88k#)ZxH&)(O+QNdLy*wO($$l=XV5M@;_?!L(*#Hnk(TO-@?Pn$jtwB%LWR;UiR)Oh^i+59|)X;_bNYQ!U{bqUS%jGGSr zaLH!XTECn(L@{5%-2CC_Wh?K7LZ8@X6_;oY*xFfT-FKQ<)R1RGL7TU&uWcx$I2;Ri%>3r77oIX%SEAqY zb=qQ&vhM@!3Dn%#Z_C;Jl8Nw@nuhBVmnTC@F`F!Jq==^CCe%K~#WEu#vq<$pM`D+J zpBx_^V{lE}mb7DKtW}AlwWtQHUBJ8ldL&8>|-E = []; const keyParts = key.split('#'); + const isWorkspaceEntry = this.workspaceLayout && keyParts[0] === this.workspaceLayout.virtualManifestName; + + // Don't add the virtual manifest (keyParts.length === 1) + // or ws childs which were not hoisted to the root (keyParts.length === 2). + // If a ws child was hoisted its key would not contain the virtual manifest name + if (isWorkspaceEntry && keyParts.length <= 2) { + continue; + } + for (let i = 0; i < keyParts.length; i++) { const key = keyParts.slice(0, i + 1).join('#'); const hoisted = this.tree.get(key); @@ -843,16 +852,26 @@ export default class PackageHoister { parts.push(keyParts[i]); } - const shallowLocs = []; - if (this.config.modulesFolder) { - // remove the first part which will be the folder name and replace it with a - // hardcoded modules folder - parts.splice(0, 1, this.config.modulesFolder); + // Check if the destination is pointing to a sub folder of the virtualManifestName + // e.g. _project_/node_modules/workspace-aggregator-123456/node_modules/workspaceChild/node_modules/dependency + // This probably happened because the hoister was not able to hoist the workspace child to the root + // So we have to change the folder to the workspace package location + if (this.workspaceLayout && isWorkspaceEntry) { + const wspPkg = this.workspaceLayout.workspaces[keyParts[1]]; + invariant(wspPkg, `expected workspace package to exist for "${keyParts[1]}"`); + parts.splice(0, 4, wspPkg.loc); } else { - // first part will be the registry-specific module folder - parts.splice(0, 0, this.config.lockfileFolder); + if (this.config.modulesFolder) { + // remove the first part which will be the folder name and replace it with a + // hardcoded modules folder + parts.splice(0, 1, this.config.modulesFolder); + } else { + // first part will be the registry-specific module folder + parts.splice(0, 0, this.config.lockfileFolder); + } } + const shallowLocs = []; info.shallowPaths.forEach(shallowPath => { const shallowCopyParts = parts.slice(); shallowCopyParts[0] = this.config.cwd; diff --git a/src/package-linker.js b/src/package-linker.js index 66ae879872..f66bda80fa 100644 --- a/src/package-linker.js +++ b/src/package-linker.js @@ -251,10 +251,6 @@ export default class PackageLinker { } else if (workspaceLayout && remote.type === 'workspace' && !isShallow) { src = remote.reference; type = 'symlink'; - if (dest.indexOf(workspaceLayout.virtualManifestName) !== -1) { - // we don't need to install virtual manifest - continue; - } // to get real path for non hoisted dependencies symlinkPaths.set(dest, src); } else {