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

Implied dependencies not found #7215

Closed
davidworkman9 opened this Issue Jun 14, 2016 · 16 comments

Comments

@davidworkman9
Contributor

davidworkman9 commented Jun 14, 2016

two problems:

devDependencies

repro:

git clone git@github.com:davidworkman9/meteorTestBugIn1.3.3.git
cd meteorTestBugIn1.3.3
meteor npm install
meteor test --driver-package practicalmeteor:mocha --port 3100

Currently getting this when trying to run tests under 1.3.3. Tried running meteor npm install has but that didn't solve it.

/Users/dave/.meteor/packages/meteor-tool/.1.3.3.ems6rm++os.osx.x86_64+web.browser+web.cordova/mt-os.osx.x86_64/dev_bundle/server-lib/node_modules/fibers/future.js:280
                                            throw(ex);
Error: Can't find npm module 'has'. Did you forget to call 'Npm.depends' in package.js within the 'modules-runtime' package?
     at Object.Npm.require (/private/var/folders/lg/zn86p4bn5m716q1wn7gjr46c0000gn/T/meteor-test-run1dmhmja/.meteor/local/build/programs/server/boot.js:203:17)
     at options.fallback (packages/modules-runtime/modules-runtime.js:21:1)
     at require (packages/modules-runtime/.npm/package/node_modules/install/install.js:88:1)
     at meteorInstall.node_modules.expect.lib.Expectation.js (node_modules/expect/lib/Expectation.js:11:1)
     at fileEvaluate (packages/modules-runtime/.npm/package/node_modules/install/install.js:153:1)
     at require (packages/modules-runtime/.npm/package/node_modules/install/install.js:82:1)
     at meteorInstall.node_modules.expect.lib.index.js (node_modules/expect/lib/index.js:3:1)
     at fileEvaluate (packages/modules-runtime/.npm/package/node_modules/install/install.js:153:1)
     at Module.require (packages/modules-runtime/.npm/package/node_modules/install/install.js:82:1)
     at Module.Mp.import (/Users/dave/.meteor/packages/modules/.0.6.2.qhcqvd++os+web.browser+web.cordova/npm/node_modules/reify/lib/runtime.js:52:16)

has is a dependency of expect, which is included in package.json under devDependencies. You see the above error on both the client and server.

after code reload

If you change devDependencies to dependencies, the error goes away until a hot code reload, after which it appears on the client. Repro: https://github.com/lorensr/meteorTestBugIn1.3.3 and video: https://youtu.be/J1pA53rZP20

From aldeed:

We are having this problem on 1.3.3, too. It happens not only when running tests, but also when running the app locally, ONLY AFTER changing a client file. On initial load, app is fine, then the first time the browser hot reloads, we get these import errors.

This does NOT have a workaround and is a severe issue.

@lorensr

This comment has been minimized.

Collaborator

lorensr commented Jun 14, 2016

Hi David, looking at the output I'm not sure what the problem is – are you able to reproduce this in an a new repo? https://github.com/meteor/meteor/blob/devel/Contributing.md#reporting-a-bug-in-meteor

@lorensr lorensr added the bug label Jun 14, 2016

@davidworkman9

This comment has been minimized.

Contributor

davidworkman9 commented Jun 15, 2016

I was able to reproduce this in a fairly small package:

https://github.com/davidworkman9/meteorTestBugIn1.3.3

meteor npm install
meteor test --driver-package practicalmeteor:mocha --port 3100
@lorensr

This comment has been minimized.

Collaborator

lorensr commented Jun 15, 2016

Thanks! I got the same error. Fixed by switching devDependencies in package.json to dependencies. The former is for eg grunt – something used during development, but not by the app code.

@lorensr lorensr closed this Jun 15, 2016

@davidworkman9

This comment has been minimized.

Contributor

davidworkman9 commented Jun 16, 2016

I'm still getting the error on the client after a test rebuild. This works prior to 1.3.3.

Everything I've ever read said that devDependencies were for things your app only needed in development. I don't need to run my unit tests in production, so I don't agree that a testing library should be in my dependencies.

@davidworkman9

This comment has been minimized.

Contributor

davidworkman9 commented Jun 16, 2016

I can confirm that using my github repo this happens as well.

  • changed devDependencies to dependencies
  • ran meteor npm install
  • ran meteor test --driver-package practicalmeteor:mocha --port 3100
  • looked at localhost:3100.
  • added a space to file.test.jsx and saved
  • when localhost:3100 rebuilt the following error was in the console.
Uncaught Error: Cannot find module 'has'
require @ install.js:85
meteorInstall.node_modules.expect.lib.Expectation.js @ Expectation.js:11
fileEvaluate @ install.js:153
require @ install.js:82
meteorInstall.node_modules.expect.lib.index.js @ index.js:3
fileEvaluate @ install.js:153
require @ install.js:82
Mp.import @ runtime.js:52
meteorInstall.imports.file.test.jsx @ file.test.jsx:1
fileEvaluate @ install.js:153
require @ install.js:82
(anonymous function) @ app.js?hash=bbe7d86…:13
@lorensr

This comment has been minimized.

Collaborator

lorensr commented Jun 16, 2016

Oops right, we're testing here 😄 Sorry!

I tried again, and still don't get the error on server or client after the change:

https://github.com/lorensr/meteorTestBugIn1.3.3

@lorensr lorensr reopened this Jun 16, 2016

@lorensr lorensr changed the title from Running tests under 1.3.3 to Dependency of a devDependency is not found during testing Jun 16, 2016

@lorensr lorensr changed the title from Dependency of a devDependency is not found during testing to Dependency of a devDependency is not found during `meteor test` Jun 16, 2016

@davidworkman9

This comment has been minimized.

Contributor

davidworkman9 commented Jun 16, 2016

Much better title thanks!

I'm not sure what you're doing differently than me then, here's a video of what I do. This is after a fresh git clone from my repo.

https://youtu.be/J1pA53rZP20

I don't think this really has a workaround yet, I can't stop the meteor test server and restart it after every change and actually get work done.

When you get to the point in the video where it's building, it completes at about 1:19.

@lorensr

This comment has been minimized.

Collaborator

lorensr commented Jun 16, 2016

Thanks for video, now I'm also seeing the client error after reload.

@lorensr lorensr changed the title from Dependency of a devDependency is not found during `meteor test` to Implied dependencies not found during `meteor test` Jun 16, 2016

@aldeed

This comment has been minimized.

Contributor

aldeed commented Jun 16, 2016

We are having this problem on 1.3.3, too. It happens not only when running tests, but also when running the app locally, ONLY AFTER changing a client file. On initial load, app is fine, then the first time the browser hot reloads, we get these import errors.

This does NOT have a workaround and is a severe issue.

@lorensr lorensr added Impact:most and removed Impact:some labels Jun 16, 2016

@lorensr lorensr changed the title from Implied dependencies not found during `meteor test` to Implied dependencies not found Jun 16, 2016

@lorensr

This comment has been minimized.

Collaborator

lorensr commented Jun 16, 2016

@benjamn looks like this is an important regression to fix

@benjamn

This comment has been minimized.

Member

benjamn commented Jun 16, 2016

These are definitely bugs that should be fixed asap. Thanks for reporting them.

In the meantime, here's a workaround for the second problem:

import "expect/node_modules/has";

Put that import declaration anywhere in your client code, and the package.json file for the has package will definitely be included.

benjamn added a commit that referenced this issue Jun 16, 2016

Fix misuse of Map object in Resolver.prototype.resolve.
Using this._resolveCache as an object rather than using the Map methods
(.has, .get, .set) almost works, miraculously, except of course when
resolving a module identifier whose name is "has", "get", or "set",
because then the cache appears to contain a matching entry, but the cached
value is a function object, not a proper byParentDir object.

Fixes #7215.

@benjamn benjamn added this to the Release 1.3.3.1 milestone Jun 16, 2016

@benjamn benjamn referenced this issue Jun 16, 2016

Merged

Release 1.3.3.1 #7237

5 of 5 tasks complete
@davidworkman9

This comment has been minimized.

Contributor

davidworkman9 commented Jun 16, 2016

Thanks @benjamn. The workaround didn't quite work for me. I included it in a client only test file, and that made it work up until it came to a file that was only being ran on the client.

But, it's not really a big deal for me because it looks like you guys will have this resolved in short order.

benjamn added a commit that referenced this issue Jun 16, 2016

Fix misuse of Map object in Resolver.prototype.resolve.
Using this._resolveCache as an object rather than using the Map methods
(.has, .get, .set) almost works, miraculously, except of course when
resolving a module identifier whose name is "has", "get", or "set",
because then the cache appears to contain a matching entry, but the cached
value is a function object, not a proper byParentDir object.

Fixes #7215.
@benjamn

This comment has been minimized.

Member

benjamn commented Jun 17, 2016

This should be fixed in Meteor 1.3.3.1, which you can try by running meteor update --release 1.3.3.1. In case you're curious why the has package was affected in this way: 3f3a6dc

@benjamn benjamn closed this Jun 17, 2016

@lorensr

This comment has been minimized.

Collaborator

lorensr commented Jun 17, 2016

Haha, that's a funny one. Thank you @davidworkman9 @aldeed for reporting! 🙌😊

@aldeed

This comment has been minimized.

Contributor

aldeed commented Jun 17, 2016

Interestingly, there are npm pkgs with all three of those names. :)

@benjamn

This comment has been minimized.

Member

benjamn commented Jun 17, 2016

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