[3/4] package/nodejs: add patch to adjust default NodeJS search path

By default, NodeJS searches global modules in /usr/lib/node, but NPM
installs them in /usr/lib/node_modules/. Therefore by default, if one
installs modules with BR2_PACKAGE_NODEJS_MODULES_ADDITIONAL, they are
not accessible by NodeJS, unless by passing a
NODE_PATH=/usr/lib/node_modules/ variable. Since this is not obvious,
and it's nicer when things work out of the box, we simply patch NodeJS
to look for modules at the right place.

See
https://stackoverflow.com/questions/15636367/nodejs-require-a-global-module-package
for some discussions on this topic.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
I am not 100% sure this is the correct approach, and I would
definitely like to get the feedback from some NodeJS people.
---
 ...ules-cjs-loader.js-adjust-default-pa.patch | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 package/nodejs/0004-lib-internal-modules-cjs-loader.js-adjust-default-pa.patch

On 2022-09-25 19:19 +0200, Thomas Petazzoni spake thusly:
> By default, NodeJS searches global modules in /usr/lib/node, but NPM
> installs them in /usr/lib/node_modules/. Therefore by default, if one
> installs modules with BR2_PACKAGE_NODEJS_MODULES_ADDITIONAL, they are
> not accessible by NodeJS, unless by passing a
> NODE_PATH=/usr/lib/node_modules/ variable. Since this is not obvious,
> and it's nicer when things work out of the box, we simply patch NodeJS
> to look for modules at the right place.
> 
> See
> https://stackoverflow.com/questions/15636367/nodejs-require-a-global-module-package
> for some discussions on this topic.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
> I am not 100% sure this is the correct approach, and I would
> definitely like to get the feedback from some NodeJS people.
> ---
>  ...ules-cjs-loader.js-adjust-default-pa.patch | 35 +++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 package/nodejs/0004-lib-internal-modules-cjs-loader.js-adjust-default-pa.patch
> 
> diff --git a/package/nodejs/0004-lib-internal-modules-cjs-loader.js-adjust-default-pa.patch b/package/nodejs/0004-lib-internal-modules-cjs-loader.js-adjust-default-pa.patch
> new file mode 100644
> index 0000000000..b4692a4e5f
> --- /dev/null
> +++ b/package/nodejs/0004-lib-internal-modules-cjs-loader.js-adjust-default-pa.patch
> @@ -0,0 +1,35 @@
> +From 4ed7de24fe18bb75b6e38922eb33141e2cec43dd Mon Sep 17 00:00:00 2001
> +From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> +Date: Sun, 25 Sep 2022 11:10:06 +0200
> +Subject: [PATCH] lib/internal/modules/cjs/loader.js: adjust default path to
> + search modules
> +
> +NPM installs modules in /usr/lib/node_modules/, but by default NodeJS
> +searches for them in /usr/lib/node/. We could also set the NODE_PATH
> +environment variable, but it is more convienient to have NodeJS
> +configured by default to find modules where they are installed.
> +
> +This issue is discussed at
> +https://stackoverflow.com/questions/15636367/nodejs-require-a-global-module-package.
> +
> +Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> +---
> + lib/internal/modules/cjs/loader.js | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
> +index f1971c40a4..52fcd56125 100644
> +--- a/lib/internal/modules/cjs/loader.js
> ++++ b/lib/internal/modules/cjs/loader.js
> +@@ -1261,7 +1261,7 @@ Module._initPaths = function() {
> +     path.resolve(process.execPath, '..') :
> +     path.resolve(process.execPath, '..', '..');
> + 
> +-  const paths = [path.resolve(prefixDir, 'lib', 'node')];
> ++  const paths = [path.resolve(prefixDir, 'lib', 'node_modules')];

It looks like this is an array, so maybe we could keep the original
path, and add the npm installation path:

    const paths = [path.resolve(prefixDir, 'lib', 'node'),
                   path.resolve(prefixDir, 'lib', 'node_modules'),];

This, in case some ot-of-tree packages simply copy files into the
original .../node/ directory.

Alternatively, we could symlink node_modules to node.

Regards,
Yann E. MORIN.

> +   if (homeDir) {
> +     ArrayPrototypeUnshift(paths, path.resolve(homeDir, '.node_libraries'));
> +-- 
> +2.37.3
> +
> -- 
> 2.37.3
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

On Sun, 25 Sep 2022 20:15:57 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> It looks like this is an array, so maybe we could keep the original
> path, and add the npm installation path:
> 
>     const paths = [path.resolve(prefixDir, 'lib', 'node'),
>                    path.resolve(prefixDir, 'lib', 'node_modules'),];
> 
> This, in case some ot-of-tree packages simply copy files into the
> original .../node/ directory.

Good point, I've sent a v2 which does exactly this.

> Alternatively, we could symlink node_modules to node.

I did consider the symlink solution as well, but I worried (perhaps
wrongly, or too conservatively) that it could get removed. Or it would
have to be the other way around: node -> node_modules, as npm installs
in node_modules and I would be worried of npm being unhappy with
installing inside a path that contains a symlink.

Overall, I found patching the search path to be safer, but if the
symlink solution is preferred, I can definitely implement it.

Best regards,

Thomas