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 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 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 install
ing 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
1 Like
What an interesting discussion
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
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