Detect "node-modules" under "packages" folder, remove them and launch reinstall from root

Description

Some users are installing things under the folder of their package which creates a “node_modules” folder.
When launching the project locally the project fails as there are confilcts between packages like in here

Possible solution

I suggest we detect if there is a “node_modules” folder under the packages folder, we remove it and we launch again automatically a install from root when we detect this from commands dev & build

We can launch internally something like

rm -rf node_modules && rm -rf **/node_modules && rm -rf package-lock.json && rm -rf yarn-error.log && rm -rf yarn.lock && npm cache clean --force && npm i

Thanks for opening this :slightly_smiling_face: I agree it can be an annoying issue. Some months ago, @luisherranz made a Pull Request to forbid local npm installs in the frontity repo, although we had to revert it.

What if, instead of creating a script to delete node_modules in the local packages, we forbid to run npm install in the local packages? I don’t know if this is even possible, but it seems it could be better because it seems to be the root of the problem.

Maybe @mmczaplinski knows better than me if it’s doable or not.

1 Like

In addition to forbidding running npm install in the local packages, we could also encourage the following packages as a way to just add dependencies to local packages

For example to add an NPM package like dayjs to a theme we could suggest doing from inside the theme folder: npx npm-add dayjs or npx adddep dayjs

And then running npm install from the root

Although a npx frontity add-dependency would be even much better (it could internally use any of these packages)

Yes, I think this should be doable using NPM lifecycle scripts. We could add a preinstall script in the package.json of our starter themes as well as any packages that are created with npx frontity create-package.

I don’t know if there are some caveats with this approach related to why we had to revert “Forbidding local npm installs”.

Perhaps @cristianbote can explain better as he reverted that change. Would there be a problem if we only add the preinstall script to package.json of mars-theme and twentytwenty-theme ?

I think this might be helpful. personally I got used to just editing package.json by hand and then running npm install but I see how that can be weird and a bit annoying.

I would open a new FD to propose this CLI command as this would be a non-trivial change.

The revert happen because the preinstall scripts are part of the package.json file, and that file get’s included in the npm packages, and that was leading to issues when someone would install frontity and the message would be printed that “you need to run install at the root”.

I’d like to add my two cents here: NPM v7 finally includes a “workspaces” feature to natively deal with multi-package scenarios (yarn had workspaces built-in for a longer time already). In a Frontity project, this allows me to add

"workspaces": [
  "./packages/*"
],

to my root package.json and be happy ever after :smiley: NPM automatically installs all packages from the packages folder, symlinks them accordingly and dedupes duplicate dependencies into the root node-modules folder. Hence I can remove all the "@frontity/mars-theme": "./packages/mars-theme"-style local references and, moreover, I can also directly npm install dependencies in a package subdirectory.
There is one drawback as of now which I have not investigated further: After running npm install whatever-dependency in a package subdirectory, I have to run npm install in the root directory again or React welcomes me with a rules-of-hooks error message on the next npm run dev. Maybe this step is required for NPM to dedupe packages again (?). This seems more like an NPM-workspaces-related problem to me, however, and could possibly also be circumvented by automatically npm installing the root package on npm run dev.

In any way, I’d like to keep my package’s node_modules folders if possible, in order to keep using the workspaces feature :slight_smile:

1 Like

What an interesting discussion :slightly_smiling_face:

We didn’t want to use workspaces in the first place because they were only supported by yarn back then but they make a lot of sense.

The current Node LTS version (v14) still includes npm 6. Does anyone know when they will start shipping npm 7?

1 Like

@luisherranz Node v15 ships with npm v7 already. For the average user, I guess, updating npm should not be a big deal (and I don’t know any reason against updating). But forcing users to use npm v7 might be less desirable :confused:

1 Like

I see, thanks @bjoluc.

So I guess v14 won’t ship with npm 7 and we will have to wait until v16 in October for a LTS version with npm 7.

1 Like