msurdi / frontend-dependencies

Copies node packages to a directory where your frontend tools will be able to find them
24 stars 5 forks source link

modulePathPrefix breaks builds #8

Closed caneta closed 6 years ago

caneta commented 6 years ago

In version 1.1.3 has been added the modulePathPrefix variable, which cause my build to break:

[frontend-dependencies]: build the "npm install" command: npm i --no-save --prefix node_modules/frontend-dependencies bootstrap-fileinput 

That causes the creation of a wrong path:

Module not found or not a directory: /home/user/myproject/node_modules/frontend-dependencies/node_modules/bootstrap-fileinput

With version 1.1.2 I had a simple

[frontend-dependencies]: build the "npm install" command: npm i --no-save bootstrap-fileinput  
[frontend-dependencies]: installing ... 
[frontend-dependencies]: copy all specified files 
[frontend-dependencies]: copy /home/user/myproject/node_modules/bootstrap-fileinput/js/fileinput.min.js to /home/user/myproject/src/main/resources/META-INF/resources/js/plugins

Should be possible to have that variable as a configurable variable inside package.json?

Thank you

msurdi commented 6 years ago

@FelixFurtmayr Do you think we really need the --prefix argument here? I don't understand why we are using it, and it seems to be causing problems. Can you help us understand why is it there?

FelixFurtmayr commented 6 years ago

The prefix is needed: It forces npm to install everything into a subfolder. Why: If the regular install and the npm install from frontend dependecies go into the same folder, npm will delete some of your needed dependencies, or won't update them. These are really tricky errors you never want to have. Separating the two npm installs by using two folders solved it.

It would be easy to add an additional parameter, but I then you will get other errors when updating the package.

"That causes the creation of a wrong path: /home/user/myproject/node_modules/frontend-dependencies/node_modules/bootstrap-fileinput "

@caneta can you tell what is wrong or does not work (what should be the expected path, looks right to me so far; what is the error)? Can you post the code from your package.json frontend-dependecies?

caneta commented 6 years ago

Hi @FelixFurtmayr, here you are a detailed description of my use case.

I'm running npm install indirectyly through a Gradle task because I work in a Liferay environment.

My package.json is the following (at the moment with exact version of frontend-dependencies to get it work):

{
  "name": "my-module",
  "version": "1.0.0",
  "scripts": {
    "postinstall": "frontend-dependencies"
  },
  "devDependencies": {
    "frontend-dependencies": "1.1.2"
  },
  "dependencies": {
    "bootstrap-fileinput": "^4.4.8"
  },
  "frontendDependencies": {
    "target": "src/main/resources/META-INF/resources/js/plugins",
    "packages": {
      "bootstrap-fileinput": {
        "src": "js/fileinput.min.js"
      }
    }
  }
}

My Gradle logs for version 1.1.2 and 1.1.3 of frontend-dependencies are

:modules:MyModule:MyModule-web:npmInstall

my-module@1.0.0 postinstall /home/user/project/workspace/modules/MyModule/MyModule-web frontend-dependencies

[frontend-dependencies]: build the "npm install" command: npm i --no-save bootstrap-fileinput
[frontend-dependencies]: installing ... [frontend-dependencies]: copy all specified files [frontend-dependencies]: copy /home/user/project/workspace/modules/MyModule/MyModule-web/node_modules/bootstrap-fileinput/js/fileinput.min.js to /home/user/project/workspace/modules/MyModule/MyModule-web/src/main/resources/META-INF/resources/js/plugins

...useless log after...


- Version **1.1.3** (**it does not work anymore!**):

...useless log before...

:modules:MyModule:MyModule-web:npmInstall

my-module@1.0.0 postinstall /home/user/project/workspace/modules/MyModule/MyModule-web frontend-dependencies

[frontend-dependencies]: build the "npm install" command: npm i --no-save --prefix node_modules/frontend-dependencies bootstrap-fileinput
[frontend-dependencies]: installing ... [frontend-dependencies]: copy all specified files Module not found or not a directory: /home/user/project/workspace/modules/MyModule/MyModule-web/node_modules/frontend-dependencies/node_modules/bootstrap-fileinput npm ERR! code ELIFECYCLE npm ERR! errno 1 npm ERR! my-module@1.0.0 postinstall: frontend-dependencies npm ERR! Exit status 1 npm ERR! npm ERR! Failed at the my-module@1.0.0 postinstall script. npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

...useless log after...



As you can see `/home/user/project/workspace/modules/MyModule/MyModule-web/node_modules/frontend-dependencies/node_modules/bootstrap-fileinput` is wrong: after build, I do not have the following directory `node_modules/frontend-dependencies/node_modules`, so fileinput.min.js connot be copied.

The correct path to me is `/home/user/project/workspace/modules/MyModule/MyModule-web/node_modules/bootstrap-fileinput/js/fileinput.min.js` as used in version 1.1.2.

Hope it helps, thank you.
FelixFurtmayr commented 6 years ago

perfect - thanks for the detailed info. package.json looks fine.

I guess the following:

Can you help debugging? I Suggest we look where the npm install is executed. We can do this with shelljs. Can you go into your node_modules_frontend-dependencies folder, modifie the index.js and have another try?

    log('installing ...')
    try {

        // here we want to know in which folder npm is executed

         shell.pwd();
         console.log(shell.pwd()); // not sure if you have to console.log the output
        // hopefully we'll know now, if this was the error

        shell.exec(npmInstallCommand);
    } catch (err) {
        fail(err);
    }

If the problem is a wrong folder path in shelljs, we might manual change directory before to ensure a correct directory.

caneta commented 6 years ago

Ok, with version 1.1.3 I've updated node_modules/frontend-dependencies/index.js as you suggested, obtaining the following (only console.log() was printed out):

# ...useless log before...

:modules:MyModule:MyModule-web:npmInstall

> my-module@1.0.0 postinstall /home/user/project/workspace/modules/MyModule/MyModule-web
> frontend-dependencies

 [frontend-dependencies]: build the "npm install" command: npm i --no-save --prefix node_modules/frontend-dependencies bootstrap-fileinput  
 [frontend-dependencies]: installing ... 
{ [String: '/home/user/project/workspace/modules/MyModule/MyModule-web']
  stdout: '/home/user/project/workspace/modules/MyModule/MyModule-web',
  stderr: null,
  code: 0,
  cat: [Function: bound ],
  exec: [Function: bound ],
  grep: [Function: bound ],
  head: [Function: bound ],
  sed: [Function: bound ],
  sort: [Function: bound ],
  tail: [Function: bound ],
  to: [Function: bound ],
  toEnd: [Function: bound ],
  uniq: [Function: bound ] }
 [frontend-dependencies]: copy all specified files 
Module not found or not a directory: /home/user/project/workspace/modules/MyModule/MyModule-web/node_modules/frontend-dependencies/node_modules/bootstrap-fileinput

# ...useless log after...

Any ideas?

FelixFurtmayr commented 6 years ago

The npm install is executed in:

"/home/user/project/workspace/modules/MyModule/MyModule-web/"

This looks correct as this is the main folder.

The modules should go in the following folder if the "prefix" option is considered, so frontend-dependencies go into this folder under node_modules:

"/home/user/project/workspace/modules/MyModule/MyModule-web/node_modules/frontend-dependencies/"

I guess for some reason the "npm install" with the prefix seems not to work:

https://github.com/npm/npm/issues/11007 https://github.com/npm/npm/issues/13933

Can you provide your npm version? If you use an early version of npm (like 2.14.x mentioned in the npm issue #13933), then an upgrade of npm might do it.

If you use a newer version like npm 4 or 5 you could once check manual if the prefix does not work by running the npm install direct in you project folder:

npm i --no-save --prefix node_modules/frontend-dependencies bootstrap-fileinput

Check if the package is installed correct into the subdir.

If not, we can put the optional variable into the code, to disable the prefix - no problem - less time than writing the comment here; unfortunately there is the big risk that npm will then remove or wont't update packages correctly. But maybe this will work with npm versions below npm 5 (as it worked perfekt for me in version 4).

FelixFurtmayr commented 6 years ago

I had to get a bit sleep ;-) What I guess would work for sure if there are any problems with npm prefix: Do the "prefix" manual:

    var npmInstallCommand = 'npm i --no-save '+ npmPackageList;
    log('build the "npm install" command: ' + npmInstallCommand)

    log('installing ...')
    try {
        // go into the prefix folder to prevent interaction between the regular npm install of the package
        shell.cd(modulePathPrefix);

        shell.exec(npmInstallCommand);

        // step two time outside the "modulePathPrefix" to get into the main folder again
        shell.cd('..');
        shell.cd('..');
    } catch (err) {
        fail(err);
    }

@caneta can you have a try if this code installs the modules correct in the prefix folder and everything else works?

caneta commented 6 years ago

Hi @FelixFurtmayr, it seems to be a Liferay issue with its Node Gradle Plugin: I'm searching for some help in their forum here.

Inside Liferay build system I have the node executable (version 8.4.0), but not npm. I have an npm wrapper named npm-cli.js in another folder path (it wraps npm version 5.3.0).

So, frontend dependencies works to me if I change line 34 of index.js

var npmInstallCommand = 'npm i --no-save --prefix '+ modulePathPrefix + ' ' + npmPackageList;

into the following

var npmInstallCommand = '../../../build/node/lib/node_modules/npm/bin/npm-cli.js i --no-save --prefix '+ modulePathPrefix + ' ' + npmPackageList;

In other words, at the moment, I need a variable to set my npm executable. But I would wait what comes from the Liferay forum before doing any changes.

FelixFurtmayr commented 6 years ago

ok, would be no problem to integrate an alternative path for npm. just tell us, what we should do when you are ready.

FelixFurtmayr commented 6 years ago

@msurdi could you close this issue as there seems to be no further problem?

msurdi commented 6 years ago

Closing. BTW, I've added you as a collaborator for the project in case you want to handle this kind of thing yourself. You've already done more work than myself in this project.

FelixFurtmayr commented 6 years ago

"I've added you as a collaborator" thank you msurdi :-)