Skip to content
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

Meteor prints jQuery missing warning #796

Closed
wavto opened this issue Nov 1, 2017 · 5 comments
Closed

Meteor prints jQuery missing warning #796

wavto opened this issue Nov 1, 2017 · 5 comments

Comments

@wavto
Copy link

wavto commented Nov 1, 2017

I'm using this awesome slider for a project based on Meteor. Since I use ReactJS in the frontend, I do not use this package directly, I got it as a dependency from this ReactJS wrapper provided by brownieboy. Moreover I use bootstrap-slider without jQuery, since I don't need jQuery for my project.

Meteor does some static code analysis for its own package bundling logic (Meteor has an own package system) which locates every require(<string literal>) in the code. When using bootstrap-slider without jQuery, Meteor prints a warning about being unable to locate jQuery. With static analysis, there is no real way of investigating if a package is really required or not, without executing the code itself. As @benjamn described here, the best way to get around this problem is to require a dynamic value containing the string literal instead of having the string directly as argument of the require function for optional packages.

That why, I would like to adapt bootstrap-slider as follow:

from

try {
  require("jquery");
}

to

try {
  var jQueryPackage = "jquery";
  require(jQueryPackage);
}

Let me provide you with a merge request containing the change.

@seiyria
Copy link
Owner

seiyria commented Nov 1, 2017

I'm not sure why it's trying to statically analyze something in an if/elseif, that seems like it would break for literally every other package that supports AMD. I'm skeptical about your patch, since it seems like:

  • it would have to be sent to every other package that supports optional AMD deps
  • static analysis shouldn't have a very tough time differentiating require('jquery') from x='jquery';require(x)

@wavto
Copy link
Author

wavto commented Nov 1, 2017

I agree on that it's not fixing the "real" problem, but technically my dynamic value solution shouldn't make a difference.

Let me correct this: Meteor don't realise when a statement is in if/else statement, it just statically collects all packages required and checks if they are installed. As I don't have jQuery on my project, I get the warning message. On the other side, Meteor will not resolve dynamic values during its static analysis, thats why my proposed change makes the difference.

I searched for AMD packages with optional dependencies without success. I'm wondering how other packages may implement optional dependencies, maybe there is an other way of doing it, which will work with Meteor, I don't know. We have have many dependent packages in the project I'm working on, but they all don't have optional dependencies. Do you have some samples of such packages?

@seiyria
Copy link
Owner

seiyria commented Nov 1, 2017 via email

@wavto
Copy link
Author

wavto commented Nov 2, 2017

I finally managed to find the boilerplate bootstrap-slider is build on. It seems, they must be more npm packages building on it.

I just opened this issue at the Meteor repository. I think, it make sense to resolve the issue at its root.

@wavto
Copy link
Author

wavto commented Nov 29, 2017

I just moved the existing issue to a feature request at meteor.

Let me close this issue, since the fix has to be done on the meteor side.

@wavto wavto closed this as completed Nov 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants