ef4 / ember-browserify

ember-cli addon for easily loading CommonJS packages from npm via browserify.
MIT License
172 stars 28 forks source link

npm-installed dependencies are not available during tests #14

Open djackson-sykes opened 9 years ago

djackson-sykes commented 9 years ago

As an example:

npm install --save-dev ember-browserify
npm install --save-dev moment

From app/router.js

import moment from 'npm:moment';

works just fine, but in any test module (e.g. tests/adapters/application.js), the same line provokes an error:

Uncaught Error: Could not find module npm:moment

Environment:

Tests using ember-cli-mocha.

ef4 commented 9 years ago

Thanks for reporting this.

I think fixing this may require changes in ember-cli proper, because I'm not seeing a hook that will let me postprocess the tests tree.

ef4 commented 9 years ago

Depends on this: https://github.com/ember-cli/ember-cli/pull/2939

Once that's merged ember-browserify 0.5.0 should fix this.

ef4 commented 9 years ago

I originally released this in 0.5.0 but hid it behind an environment variable in 0.6.0 because I don't want to break anybody who upgrades prematurely before ember-cli releases.

djackson-sykes commented 9 years ago

Thanks for being so quick with this!

ef4 commented 9 years ago

I was about to enable this by default now that the ember-cli change shipped, but I found a bug.

This browserifies the app and test trees separately, which results in two copies of any module that's used in both. If you expected them to share state, you can have nasty bugs.

Working on a fix.

ef4 commented 9 years ago

I haven't had a chance to fix this yet. The functionality is deployed and available for people who want it, but you need to turn it on by setting the environment variable BROWSERIFY_TESTS=true. The only caveat is the bug I mentioned above -- you get two independent copies of any module that you import from both app and tests.

eoneill commented 9 years ago

What work needs to happen for this to land without the BROWSERIFY_TESTS flag? This has bitten us a few times and it's potentially a blocking issue for our build environment.

Thanks!

ef4 commented 9 years ago

To do this correctly we'd need to crawl both the app and vendor trees to gather dependencies and then call browserify only once, so that we don't get duplicate modules. Instead of running browserify separately for each tree.

johnnyshields commented 9 years ago

:+1: for this. Browserify is the way forward, importing its modules is so nice but need it in tests.

mgenev commented 9 years ago

this module is brilliant, i'm also very much hoping to be able to use this in my tests

xkb commented 9 years ago

@ef4: can I throw a PR at you to move BROWSERIFY_TESTS=true to a flag in environment.js?

johnnyshields commented 9 years ago

@ef4 pinging on this. We really would like first-class browserify support for Ember, and w/out this issue resolved we are having messy hacks to get our tests to work.

ef4 commented 9 years ago

The ember-cli folks are working on a big internal refactor that will make ember-browserify-like functionality first-class.

johnnyshields commented 9 years ago

@ef4 is there an issue I can follow on ember-cli for this?

ef4 commented 9 years ago

https://github.com/ember-cli/ember-cli/issues/4211

rupurt commented 9 years ago

Thanks for the workaround @xkb & @ef4.

I'm still running into issues unfortunately :( I've confirmed that L28 is getting executed.

I get the following error when trying to import sinon-chai for ember-mocha.

Error: Could not find module `npm:sinon-chai` imported from `my-project/tests/unit/controllers/delete-activity-test`

via

import sinonChai from "npm:sinon-chai";
jonnedeprez commented 8 years ago

As a temporary solution I'm importing the package indirectly through a dummy file in the app tree:

app/helpers/my-cool-npm-package.js

import myCoolNpmPackage from 'npm:my-cool-npm-package';
export default myCoolNpmPackage;

tests/unit/models/product.js

import myCoolNpmPackage from 'hello-world/helpers/my-cool-npm-package'

Works fine.

oliverwilkie commented 8 years ago

@jonnedeprez does this mean that my-cool-npm-package is bundled into production builds? I have an npm-dependency that I need to use only in my tests.

dwickern commented 8 years ago

@oliverwilkie You can manually exclude the package from production builds

// config/environment.js
module.exports = function(environment) {
  var ENV = {
    environment: environment,
    baseURL: '/',
    locationType: 'auto',

    // ...

    browserify: {
      // your browserify options if you have any
      ignores: [ ]
    },
  };

  if (environment === 'production') {
    ENV.browserify.ignores.push('my-cool-npm-package');
  }

  return ENV;
};
johnnyshields commented 8 years ago

@stefanpenner curious as to why you closed this issue? Is there a linked issue that resolves this?

ef4 commented 8 years ago

Yeah, as far as I know there is still a legit unresolved issue here. Trying to use ember-browserify from the tests tree is likely to troll people. The TESTS flag described above is not a full solution, just a different bad behavior that is preferable for some use cases.

izelnakri commented 7 years ago

any updates on this?