Seravo / wordpress

The WordPress project layout used by many of Seravo's customers, suitable also for local development with Vagrant and git deployment
https://seravo.com
GNU General Public License v3.0
102 stars 54 forks source link

Add the .nvmrc file to facilitate the use of nvm. #138

Closed Kimitri closed 3 years ago

Kimitri commented 4 years ago

With .nvmrc the Node Version Manager (nvm) can be used to manage the NodeJS version dependency. Dependencies introduced in the package.json file have an implicit restriction on which NodeJS version(s) they can be used. The .nvmrc file makes this restriction explicit.

After installing nvm you can make use of this file by executing the following commands:

$ nvm install
$ nvm use

This installs and activates the correct NodeJS version for the project. After this npm install and gulp should work without a hitch.

ottok commented 4 years ago

You now set the version at 10?

vagrant@development:/data/wordpress$ nodejs --version
v12.13.1

I wonder how we should roll-out updates of next NodeJS version if there is a hard coded version string in the project?

Kimitri commented 4 years ago

Yes, the .nvmrc file sets the NodeJS version to 10.16.0 when nvm is used (nvm install and nvm use). There already exists a package-lock.json file in the project that locks the versions of all the NodeJS modules that this project depends on. To make sure those dependencies work correctly, we need a way to specify the NodeJS version on which they were meant to run. This is provided by the .nvmrc file.

The need for this arose when I tried to install the NodeJS modules to run Gulp. libsass fails to compile on macOS when attempting to install this version of the node-sass package on newer versions of Node (v. 12.12.0 in my case). Here are the errors thrown by node-gyp:

../src/create_string.cpp:17:25: error: no matching constructor for initialization of 'v8::String::Utf8Value'
  v8::String::Utf8Value string(value);
                        ^      ~~~~~
/Users/kimmo/.node-gyp/12.12.0/include/node/v8.h:3046:5: note: candidate constructor not viable: no known conversion from 'v8::Local<v8::Value>' to
      'const v8::String::Utf8Value' for 1st argument
    Utf8Value(const Utf8Value&) = delete;
    ^
/Users/kimmo/.node-gyp/12.12.0/include/node/v8.h:3039:5: note: candidate constructor not viable: requires 2 arguments, but 1 was provided
    Utf8Value(Isolate* isolate, Local<v8::Value> obj);
    ^
1 error generated.
make: *** [Release/obj.target/binding/src/create_string.o] Error 1
gyp ERR! build error 
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/Users/kimmo/temp/kh-wordpress/node_modules/node-gyp/lib/build.js:262:23)
gyp ERR! stack     at ChildProcess.emit (events.js:210:5)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:272:12)
gyp ERR! System Darwin 18.7.0
gyp ERR! command "/Users/kimmo/.nvm/versions/node/v12.12.0/bin/node" "/Users/kimmo/temp/kh-wordpress/node_modules/node-gyp/bin/node-gyp.js" "rebuild" "--verbose" "--libsass_ext=" "--libsass_cflags=" "--libsass_ldflags=" "--libsass_library="
gyp ERR! cwd /Users/kimmo/temp/kh-wordpress/node_modules/node-sass
gyp ERR! node -v v12.12.0
gyp ERR! node-gyp -v v3.8.0
gyp ERR! not ok 
Build failed with error code: 1
npm WARN seravo-wordpress@1.3.0 No license field.

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! node-sass@3.13.1 postinstall: `node scripts/build.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the node-sass@3.13.1 postinstall script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/kimmo/.npm/_logs/2020-05-29T18_28_22_174Z-debug.log

As is visible here, the V8 runtime is not compatible with the header files provided by libsass, which means that there is a mismatch between the runtime version expected by libsass and the one provided by NodeJS 12. This can be remedied by using an older version of NodeJS - in this case version 10.16.0 seems to do the trick.

The only way to have at least some chance of providing reproducible builds across different environments with NodeJS is to lock down both the package dependecy versions and the runtime version. Locking down package dependency versions simply isn't enough.

If I understood correctly, you expect the Gulp workflow to be executed on the VM, right? I see this as a complication - especially when considering running this stuff ın Docker containers. Also, I think it would be good to separate the development workflow from the execution context to better emulate the actual production environment - it is, after all, a kind of deployment target.

I'm not sure I understand your comment on rolling out updates. It is, of course, possible to run nvm on the VM. The fact that the VM has a newer version of NodeJS installed by default should cause no problems.

Sorry for such a long comment! :)

ottok commented 3 years ago

I think the root problem here is that you were unable to run gulp/node-sass due to some other reason, maybe outdated dependencies. There is one gulp/node-sass already preinstalled, as well is the NodeJS LTS version. They are broadly compatible and allow for smooth upgrades. If one needs to revert to downgrading NodeJS then it risks causing problems for everything else that is installed.

Related: https://github.com/Seravo/wordpress/issues/128#issuecomment-635560195

Kimitri commented 3 years ago

Hmm... Nope. The reason is the one I described earlier but I guess the way to go is just run Gulp on the VM. As I pointed out, I ran into the problem when I was setting up the asset pipeline to run on the macOS host, not the guest VM. (I usually prefer running asset pipelines natively for performance, transparency and portability reasons)

Regarding screwing things up with nvm: Using nvm is actually really safe as it installs packages in a version-specific container. You use nvm to automatically switch between multiple NodeJS runtime versions. It's not downgrading, It's dependency management.

ottok commented 3 years ago

OK, thanks for the feedback. We'll consider it.

frimro commented 3 years ago

Continuing this in #186