Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

node_modules folder inside host, not symlinked #33

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Conversation

Mte90
Copy link
Member

@Mte90 Mte90 commented Jan 27, 2023

Following hashicorp/vagrant#11842

In this way NPM inside develop works again inside VVV.
I am not sure if the folder path is right or not.

Still testing at 100%

@Mte90
Copy link
Member Author

Mte90 commented Jan 27, 2023

To me worked everything now :-)

provision/vvv-init.sh Outdated Show resolved Hide resolved
provision/vvv-init.sh Outdated Show resolved Hide resolved
provision/vvv-init.sh Outdated Show resolved Hide resolved
provision/vvv-init.sh Outdated Show resolved Hide resolved
@Mte90
Copy link
Member Author

Mte90 commented Feb 2, 2023

I was just thinking that this mount point doesn't persist on a new boot of the VM so every time is required a provision.
We don't have anything that let us to add a script on boot for this right?

@tomjn
Copy link
Member

tomjn commented Feb 2, 2023

if it's in /tmp yes but we've put this in ~/.vvv so it's on the virtual disk image? That does mean a vagrant destroy would remove those folders though

@Mte90
Copy link
Member Author

Mte90 commented Feb 7, 2023

No I was talking about the mount command, that it will be kept just on that execution but it won't be persist at the next reboot.

@Mte90
Copy link
Member Author

Mte90 commented Mar 22, 2023

Any updates for this?

@tomjn
Copy link
Member

tomjn commented Mar 22, 2023

I don't have the bandwidth to look at this, it may be several weeks before of time before it bubbles to the top of the queue and I can look at it myself ( VVV and non-VVV ).

@tomjn
Copy link
Member

tomjn commented Mar 23, 2023

also how does this handle the user running commands on the host not the guest? Is the node_modules not visible in the host because it's a symlink to a VM only folder?

@Mte90
Copy link
Member Author

Mte90 commented Mar 23, 2023

The host cannot see the folder as it is symlinked inside the VM, just see a file with the symlink (on linux) to a folder that not exists in the host.

part that the other issue is the mount that we need in VVV otherwise it is required to do it everytime a provision.

@tomjn
Copy link
Member

tomjn commented Mar 23, 2023

would it make more sense to instead tell npm to use a node_modules folder that lived somewhere else, rather than symlinking it? I can see symlinks causing problems on Windows as they require adminsitrator priviledges on a lot of Windows versions

@Mte90
Copy link
Member Author

Mte90 commented Mar 23, 2023

The problem is that npm has issues on using a shared folder for node_modules, so if it is inside the machine everything works.

@tomjn
Copy link
Member

tomjn commented Mar 23, 2023

we don't know for sure if someone will use the host or the guest though, it either needs to work with both, or be configurable

@Mte90
Copy link
Member Author

Mte90 commented Mar 23, 2023

Well it will never work anyway.

If in the host I have nodejs 23 and VVV has nodejs 12 something will not work. that was basically the reason why I had those problems otherwise I executed npm in the host machine and not inside VVV.

@Mte90
Copy link
Member Author

Mte90 commented Mar 23, 2023

Ok so seems that on linux host the folder get shared also if it is from inside the machine:
immagine
When will finish I can try to execute npm from the host and see what happens.

@tomjn
Copy link
Member

tomjn commented Mar 23, 2023

a lot of people will use watchers on the host as it's more reliable. NVM is installed inside the VM so users can switch to the appropriate node version, likewise they could do that on the host.

node_modules can be moved without a symlink via https://stackoverflow.com/questions/14742553/how-to-set-custom-location-for-local-installation-of-npm-package

Note there are also other consequences, those packages won't be indexable by editors and tools on the host.

At a minimum this would need to be optional, in addition to node modules, watchers don't work well on shared folders and that applies to core itself not just the node modules folder

Stack Overflow
Is it possible to specify a custom package destination for npm install, either through a command flag or environment variable?

By default, npm local installs end up in node_modules within the curr...

@Mte90
Copy link
Member Author

Mte90 commented Mar 23, 2023

So using npm from the host machine:
immagine

It is possible to not execute NPM from VVV in this template, there is an option I did few months ago.

The problem is that the nodejs version compatible is very old and VVV has the right one so creates a lot of troubles.
Anyway grunt was able to compile it and watch works, it is just the installation procedure that requires that nodejs specific version.

just execute npm build once
@tomjn
Copy link
Member

tomjn commented Mar 23, 2023

The problem is that the nodejs version compatible is very old and VVV has the right one so creates a lot of troubles.

This is all solved by nvm for both the host and the guest VM

@Mte90
Copy link
Member Author

Mte90 commented Mar 23, 2023

Well I don't have nvm locally and don't want to install it just to have a specific nodejs version for wordpress.

@tomjn
Copy link
Member

tomjn commented Mar 23, 2023

arguably it's better to install NodeJS only through nvm or a similar tool. nvm use will read the .nvmrc in the folder and install and set the appropriate Node version in the current shells path. It's far more reliable than installing NodeJS at a system level.

Even core has support for it: https://github.com/WordPress/wordpress-develop/blob/trunk/.nvmrc

It's also the secret sauce that lets VVV always have the correct Node version for core:

https://github.com/Varying-Vagrant-Vagrants/custom-site-template-develop/blob/master/provision/vvv-init.sh#L119-L122

GitHub
WordPress Develop, Git-ified. Synced from git://develop.git.wordpress.org/, including branches and tags! This repository is just a mirror of the WordPress subversion repository. Please include a li...
GitHub
A VVV site template for working on WordPress Core development. For client development use the custom-site-template instead - custom-site-template-develop/vvv-init.sh at master · Varying-Vagrant-Vag...

@tomjn
Copy link
Member

tomjn commented Jun 7, 2023

I've tried to fix a merge conflict, @Mte90 for this there should be an option, I can see it being useful for some people while breaking things for others.

I'd prefer things like this be trialled as parameters then promoted to defaults once we're sure all is good

@Mte90
Copy link
Member Author

Mte90 commented Jun 8, 2023

Well as today isn't good anyway as it is a mess to use npm inside a VM. It uses all the ram and crash at the end also with a symlinked folder, we should try yarn or pnpm to fix the issue at the top.
But I lost my interest in contributing to wordpress as it is required to compile with npm also if you are not interested on working in the JS part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants