Yoast / javascript

Monorepo for all the JavaScript within Yoast
131 stars 54 forks source link

Dependencies added to packages in monorepo cannot be resolved when building premium #291

Closed maartenleenders closed 5 years ago

maartenleenders commented 5 years ago

Explanation

When adding a dependency to one of the monorepo's packages, linking the monorepo to premium, and then building premium, the build fails:

    ERROR in ./node_modules/@yoast/components/src/WordCloud.js
    Module not found: Error: Can't resolve 'react-tag-cloud' in '/Users/maarten/Yoast/VVV/www/wordpress-one/public_html/wp-content/plugins/wordpress-seo-premium/node_modules/@yoast/components/src'
     @ ./node_modules/@yoast/components/src/WordCloud.js 13:11-25
     @ ./node_modules/@yoast/components/src/index.js
     @ ./js/src/components.js

The example error came about by adding react-tag-cloud to the components package:

yarn add react-tag-cloud

adding TagCloud to some js file (in this case WordCloud but you can also add it to WordList.js and simply console.log it somewhere in there to use it:

import TagCloud from "react-tag-cloud";

Then, I went to the root of the monorepo, and ran

yarn install
yarn lerna bootstrap

Then, in premium, I added the monorepo and tried to build premium

yarn link-monorepo
composer install && yarn install && grunt build
cd premium && yarn install && grunt build

That produces the error above. I've tried adding other packages at random together with @bintzandt and they also produced the same error.

Adding the package to wordpress-seo-premium's package.json does allow the build to pass, but I don't think this should be required. My running theory on why this does not happen with other packages is that those packages have coincidentally pre-existed in premium's package.json or as a sub-dependency of packages required therein.

I expected to be able to add the dependency to the package, and that it works in a plug-and-play way in premium.

atimmer commented 5 years ago

tl;dr: This is an issue with how yarn works. @herregroen and I couldn't come up with a quick solution, so we would need to expend more time and introduce more complexity to fix this. The workaround is simple: temporarily use yarn add react-tag-cloud in wordpress-seo(-premium). Because of those two factors we decided to not fix this issue. We've also considered that it is not necessarily a bad thing for the barrier on adding third party dependencies to be a little bit higher.

Now for the explanation why it doesn't work. After your yarn add react-tag-cloud the folder structure looks like this:


 /wordpress-seo                                            
     /node_modules                                          
         /@yoast                                            
             /components (symlink) ────────────────────────┐
                 /src                                      │
                     /file-requiring-react-tag-cloud.js    │
                 /package.json                             │
             /yoastseo (symlink)   ──────────────────┐     │
                 /package.json                       │     │
                                                     │     │
 /javascript                                         │     │
     /packages                                       │     │
         /components ◀───────────────────────────────┼─────┘
             /src                                    │      
                 /file-requiring-react-tag-cloud.js  │      
             /package.json                           │      
         /yoastseo              ◀────────────────────┘      
             /package.json                                  
     /node_modules                                          
         /react-tag-cloud                                   

When the file-requiring-react-tag-cloud.js is trying to require react-tag-cloud it doesn't find that package inside the tree, because it doesn't actually exist in that tree. This is the case even though the package is added to the package.json of the components package. The (hypothetical) reason yarn doesn't install this into the root node_modules of wordpress-seo is because yarn looks at the dependencies for @yoast/components as it retrieved them from the npm registry instead of looking at the 'live' package.json.