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 in subdirectory? #210

Closed
DenisGorbachev opened this Issue Jul 23, 2015 · 10 comments

Comments

Projects
None yet
4 participants
@DenisGorbachev

Reproduction steps:

Results:

  • mocha works
  • Wallaby doesn't work (Error: Cannot find module 'object-path')

That's because Wallaby only uses the top-level node_modules, while ignoring all node_modules (both top-level and inside core/).

In theory, I could install "core" as npm package itself. However, I'd like to keep it as git submodule, because I develop core along with the main package.

Is it possible to utilize all node_modules, not just top-level ones?

@ArtemGovorov

This comment has been minimized.

Show comment
Hide comment
@ArtemGovorov

ArtemGovorov Jul 23, 2015

Member

It is possible, you may just give wallaby a hint where to look for node modules:

module.exports = function(wallaby) {
  process.env.NODE_PATH = 
    require('path').join(wallaby.localProjectDir, 'core', 'node_modules');
  ...
}

I understand it's not perfect and it'd be better if it would just work as in mocha (actually mocha with any code coverage solution should fail for the same reason, as it also has its own cache of instrumented files where it will not copy nested node modules, unless instructed to do so). So as opposed to mocha that is using a local folder, wallaby is using its own cache and doesn't copy node modules there, so needs a hint where to find node modules. Top level modules are automatically handled, but modules in subfolders may need scanning to find them, which will be a bit less performant than the proposed solution with NODE_PATH.

P.S You may consider adding !core/node_modules/** to your files list, right now all coffee script files from the node modules are copied to the wallaby cache.

Member

ArtemGovorov commented Jul 23, 2015

It is possible, you may just give wallaby a hint where to look for node modules:

module.exports = function(wallaby) {
  process.env.NODE_PATH = 
    require('path').join(wallaby.localProjectDir, 'core', 'node_modules');
  ...
}

I understand it's not perfect and it'd be better if it would just work as in mocha (actually mocha with any code coverage solution should fail for the same reason, as it also has its own cache of instrumented files where it will not copy nested node modules, unless instructed to do so). So as opposed to mocha that is using a local folder, wallaby is using its own cache and doesn't copy node modules there, so needs a hint where to find node modules. Top level modules are automatically handled, but modules in subfolders may need scanning to find them, which will be a bit less performant than the proposed solution with NODE_PATH.

P.S You may consider adding !core/node_modules/** to your files list, right now all coffee script files from the node modules are copied to the wallaby cache.

@DenisGorbachev

This comment has been minimized.

Show comment
Hide comment
@DenisGorbachev

DenisGorbachev Jul 24, 2015

Thanks for the tips, it works!

Here's my final wallaby.js: https://gist.github.com/DenisGorbachev/c370efdee4acec46b83e

Thanks for the tips, it works!

Here's my final wallaby.js: https://gist.github.com/DenisGorbachev/c370efdee4acec46b83e

@DenisGorbachev

This comment has been minimized.

Show comment
Hide comment
@DenisGorbachev

DenisGorbachev Jul 24, 2015

By the way, maybe it would be better to update the docs with these instructions?

Plus, a micro-pull-request:

- process.env.NODE_PATH = require('path').join(wallaby.localProjectDir, 'core', 'node_modules');
+ process.env.NODE_PATH += ':' + require('path').join(wallaby.localProjectDir, 'core', 'node_modules');

By the way, maybe it would be better to update the docs with these instructions?

Plus, a micro-pull-request:

- process.env.NODE_PATH = require('path').join(wallaby.localProjectDir, 'core', 'node_modules');
+ process.env.NODE_PATH += ':' + require('path').join(wallaby.localProjectDir, 'core', 'node_modules');
@ArtemGovorov

This comment has been minimized.

Show comment
Hide comment
Member

ArtemGovorov commented Jul 25, 2015

@DinisCruz

This comment has been minimized.

Show comment
Hide comment
@DinisCruz

DinisCruz Jul 26, 2015

that worked really well, I'm still in early stages of integrating wallaby on my project so my wallaby.js is still quite simple:

module.exports = function (wallaby) {
  process.env.NODE_PATH = require('path').join(wallaby.localProjectDir, 'TM_Website', 'node_modules');

  //console.dir(wallaby)

  return {
    files: [
      "TM_Website/src/**/*.coffee",
      "TM_Jade/**/*.jade"
    ],

    tests: [
      "TM_Website/test/**/*.coffee"
    ],

    env: {
      type: 'node',
    }
  };
};

that worked really well, I'm still in early stages of integrating wallaby on my project so my wallaby.js is still quite simple:

module.exports = function (wallaby) {
  process.env.NODE_PATH = require('path').join(wallaby.localProjectDir, 'TM_Website', 'node_modules');

  //console.dir(wallaby)

  return {
    files: [
      "TM_Website/src/**/*.coffee",
      "TM_Jade/**/*.jade"
    ],

    tests: [
      "TM_Website/test/**/*.coffee"
    ],

    env: {
      type: 'node',
    }
  };
};
@ArtemGovorov

This comment has been minimized.

Show comment
Hide comment
@ArtemGovorov

ArtemGovorov Jul 31, 2015

Member

@DenisGorbachev quick note on using ':' + require('path').join..., I have corrected the example in docs to make it cross-platform:

process.env.NODE_PATH += path.delimiter 
  + path.join(wallaby.localProjectDir, 'core', 'node_modules');
Member

ArtemGovorov commented Jul 31, 2015

@DenisGorbachev quick note on using ':' + require('path').join..., I have corrected the example in docs to make it cross-platform:

process.env.NODE_PATH += path.delimiter 
  + path.join(wallaby.localProjectDir, 'core', 'node_modules');
@DenisGorbachev

This comment has been minimized.

Show comment
Hide comment
@DenisGorbachev

DenisGorbachev Jul 31, 2015

Ah, totally forgot about Windows :) Thanks!

On Fri, Jul 31, 2015 at 5:41 AM Artem Govorov notifications@github.com
wrote:

@DenisGorbachev https://github.com/DenisGorbachev quick note on using ':'

  • require('path').join..., I have corrected the example in docs to make
    it cross-platform:

process.env.NODE_PATH += path.delimiter

  • path.join(wallaby.localProjectDir, 'core', 'node_modules');


Reply to this email directly or view it on GitHub
#210 (comment).

Ah, totally forgot about Windows :) Thanks!

On Fri, Jul 31, 2015 at 5:41 AM Artem Govorov notifications@github.com
wrote:

@DenisGorbachev https://github.com/DenisGorbachev quick note on using ':'

  • require('path').join..., I have corrected the example in docs to make
    it cross-platform:

process.env.NODE_PATH += path.delimiter

  • path.join(wallaby.localProjectDir, 'core', 'node_modules');


Reply to this email directly or view it on GitHub
#210 (comment).

@szarouski

This comment has been minimized.

Show comment
Hide comment
@szarouski

szarouski Sep 4, 2015

Probably a silly question, but how wallaby should be defined?

Probably a silly question, but how wallaby should be defined?

@ArtemGovorov

This comment has been minimized.

Show comment
Hide comment
@ArtemGovorov

ArtemGovorov Sep 4, 2015

Member

@szarouski wallaby in the context of this issue is a parameter passed to the main configuration function of your wallaby config:

module.exports = function(wallaby) { ...

Does it answer your question? If not, could you please clarify a bit what do you mean by defined?

Member

ArtemGovorov commented Sep 4, 2015

@szarouski wallaby in the context of this issue is a parameter passed to the main configuration function of your wallaby config:

module.exports = function(wallaby) { ...

Does it answer your question? If not, could you please clarify a bit what do you mean by defined?

@szarouski

This comment has been minimized.

Show comment
Hide comment
@szarouski

szarouski Sep 4, 2015

Yes, this is it. Thanks Artem.

Yes, this is it. Thanks Artem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment