search

hpcc-systems / Visualization

HPCC JavaScript Framework
https://hpcc-systems.github.io/Visualization/
Other
92 stars 62 forks source link

Should remove RequireJS dependencies from bower.json? #712

Closed buunguyen closed 9 years ago

buunguyen commented 9 years ago

I'm not sure if this has been discussed: since hpcc-viz can work in non-AMD and even non-RequireJS environments, its bower.json shouldn't define dependencies to RequireJS, RequireJS-CSS, RequireJS-Plugins. Otherwise, in non-AMD projects, these dependencies are unnecessarily pulled to the project after every bower install. In addition, we have to manually exclude them to be wired to the page as we use automatic script injection mechanism.

GordonSmith commented 9 years ago

...maybe - my goal is to make it as easy as possible for both the AMD and the non AMD folks. Technically the dist-amd folder doesn't use the bower_dependencies at all, since its included all dependencies into the bundles (including requireJS) - moving it to package.json will probably satisfy me from a developers point of view. (I am worried I am missing something obvious)

buunguyen commented 9 years ago

In non-AMD: we certainly don't want bower install to pull over RequireJS etc. In AMD: from what you said, we don't want to do that either because RequireJS is already bundled. (Even if it's not bundled, I don't think we should include because hpcc-viz widgets are UMD.)

So my vote to remove RequireJS from bower.json :).

I'm not sure about moving to package.json though. Currently package.json only declares dep dependencies, e.g. packages required to build/test hpcc-viz. Do we need RequireJS deps to be pulled to node_modules to build/test? Do the users when they install to their project need that?

GordonSmith commented 9 years ago

Actually its already there and it is very much a dev dependency (I will need to add the plugins) - you were already using it for your build when I added my stuff! Also developing the widgets without require would be too slow, especially for things like the dermatology test page.

buunguyen commented 9 years ago

You're right. I don't remember why they weren't added to package.json as opposed to bower.json given they're dev, not lib, dependencies. So might be moving to package.json is the right way to go.

mzummo commented 9 years ago

@GordonSmith I think this is resolved

GordonSmith commented 9 years ago

yup