ef4 / ember-browserify

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

`ember test` breaks when `ember-simple-auth` is installed with Ember CLI 1.13.11 #53

Closed XrXr closed 8 years ago

XrXr commented 8 years ago

I recently tried to update to Ember CLI 1.13.11 and got error

ENOENT: no such file or directory, stat '/Users/alan/test/EmberBrowserifySimpleAuthBug/tmp/concat_with_maps-input_base_path-iLT0hOJN.tmp/0/browserify-tests/browserify.js

When running ember test.

I suspect this is because ember-simple-auth also depends on ember-browserify. I tried depending on the dependencies from ember-simple-auth in my app, but it doesn't work.

Is this because only npm imports in /app of add-ons work?

Here is a reproduction repo

asakusuma commented 8 years ago

It's a known issue any npm imports must be imported from the /app directory. However, as long as the same npm modules are imported in /app, you can use them in /addon.

Additionally, if you have an App which depends on Addon, which depends on an npm module imported via ember-browserify, the App must declare the npm module in it's package.json.

See the docs for more info: https://github.com/ef4/ember-browserify#the-workaround

XrXr commented 8 years ago

I don't think that is the issue. I installed the dependencies ember-simple-auth imports through ember-browserify, and the same error happens.

Additionally, this is not an issue on Ember CLI 1.13.8

asakusuma commented 8 years ago

You're right, this seems to be a compat issue with the new ember-cli. I'm trying to figure out the root cause, will report back when I find something.

XrXr commented 8 years ago

I tracked down the problem.

ember-simple-auth has browserify: {tests: true} in its config/environment. This config gets merged with the host app by default. That option triggers importing browserify-tests/browserify.js, which does not exists.

The reason that simple auth has that flag on is only for its own tests, not for test merging into host apps.

I suspect this worked in older versions of Ember CLI because the timing of removal was different, so that file was still reachable when imported.

The fix for this is simple: simple-auth makes changes so its config is not merged into the host, or ember-browserify only check for that flag in the host app/addon.

I'm not sure which solution is better. Thoughts?

duboff commented 8 years ago

Same issue, think the simple-auth change makes more sense.

asakusuma commented 8 years ago

@XrXr @duboff thanks for digging deeper.

Unfortunately, I don't think that's the root issue. If you checkout ember-simple-auth and bump ember-cli and then run the tests, you get the same issue.

I think there's a core ember-cli issue where a file added via postprocessTree() is not available in included(). I thought I had addressed this issue earlier, but things may have changed. I'll keep working on this.

kenips commented 8 years ago

thanks @asakusuma! Have you opened an issue on ember-cli side to track this?

kenips commented 8 years ago

Spoke too soon I guess - this is https://github.com/ember-cli/ember-cli/issues/4982?

asakusuma commented 8 years ago

@kenips that issue presented itself with the same error, but I think there's a different root issue. @chadhietala (ember-cli owner) and I are currently debugging this to figure out root cause. Once we figure out the problem we will file a ticket.

asakusuma commented 8 years ago

We're trying to figure out if this is an ember-cli problem, or if the ember-cli bump is just exposing an ember-browserify problem

asakusuma commented 8 years ago

We've identified the problem: the API for ember-cli's app.import has changed. We're working on a solution that works for both 1.13.8 and versions after.

asakusuma commented 8 years ago

@kenips @duboff @XrXr just released a fix in 1.1.2. Let me know if it works for you guys, it appears to be fixed, but I know there are a lot of variables with combinations of addons and host apps with ember-cli versions.

asakusuma commented 8 years ago

Actually, since our fix branches on ember-cli versions, it won't work for range versions. Working on a fix.

kenips commented 8 years ago

@asakusuma thanks for the update. It's not working for me with 1.1.3 right off the bat. Let me check if there's something else causing it...

kenips commented 8 years ago

The version detection isn't working for me unfortunately, ember-cli-version-checker works though so I've opened #57 at your disposal. Thanks!

asakusuma commented 8 years ago

Thanks @kenips! 1.1.4 released.

XrXr commented 8 years ago

1.1.4 works for me. Thank you!

asakusuma commented 8 years ago

Great, thanks for reporting the issue! Closing.