-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add package manager files to this repo. #199
Comments
#122 Also covers this to some extent. As I note there, I need to create some time to looking into the best way of doing this for all of the DataTables repos. |
Okay, let me know if there is something I can assist with! |
This is something we should look at it. We've got repos (like this one) which are currently on github and sadly published to npm and the author clearly doesn't have an understanding of CommonJS. Imho, and I do understand CommonJS ( at least somewhat. ;) ) we need to break up the Plugins Repo to one repo per plugin. Each plugin needs a So in CommonJS we can do something something like...
Or, in the event we need to "plugin" two DataTables/jQuery plugins...
etc. Opinions? @DataTables I could whip up an example. |
The DataTables integration files for the styling libraries are shortly going to be moved into the DataTables main repo (actually they are already there, but it hasn't been publicly announced). |
I don't see them in the DataTables repo. How do we use these libraries? When you say styling libraries, are you referring to just the CSS or the jQuery integration that permits things like bootstrap paging widgets? |
else if ( typeof exports === 'object' ) {
// Node/CommonJS
factory( require('jquery'), require('datatables') );
} |
Hi,
Yes. Bootstrap, Foundation and jQuery UI.
Ah yes - I've not code them in the built repo yet - just the source one. To use you would include the DataTables JS core file, the styling JS file you need and the CSS stylesheet for the styling framework you want. I wrote a blog post explaining the file structure for DataTables and all of its extensions. My biggest challenge is supporting multiple styling frameworks from multiple plug-ins for DataTables. I had thought that the new file structure was a good way of doing it, and importantly maintainable. The old method was not at all. Also I don't particularly want to maintain a repo for each styling integration for each extensions (that would be 48 repos!) - hence the approach I've taken. Proper support for Bowserify and other development tools is something I'm very keen to fully add. I would very much appreciate any feedback you have on how I can improve my code for that. |
The only way I know of is to break out your repos. It seems inevitable from standpoint, and the resistance to it seems kind of interesting. ;) It's easier to maintain, not more difficult. You can also better delegate to others under a more atomistic repo. This isn't SVN either, it's git. NPM and co will want the dev repos in the manifest -- what will you do? Point them all here? How will you manage separate Just look at datetime-moment. That will require moment.js. How will you handle making
I would also strongly consider
|
Ok.. so looking at the CommonJS code, it's in worse shape than I originally thought. So what we do here is essentially expand on a DataTable object with jQuery. We need an instance of jQuery to do that expansion, but we don't actually modify that version of jQuery, we just modify an instance of DataTables. That's not all that useful because in CommonJS we need to modify the instance of DataTables that an extended version of jQuery is actually using -- the only thing that modifies an instance of jQuery is actually DataTables.js. We do modify the version of jQuery in DataTables proper.. So, that'll have to be modified too if you want to get CommonJS working. So for instance, you'll have to return a version of jQuery there. So how the user interface will have to look.. // Here we used a shared instance of jQuery and expand it with DataTables.
var $ = require('jQuery');
var dt$ = require('datatables')($);
// Or, alternatively, let it get it's own copy of jQuery.
var dt$ = require('datatables')();
// Now the kicker.. To do the extension of DataTables object, we'll have to pass in $.
var datatablesPluginBootstrap3 = require('datatables-plugin-bootstrap3');
var dtpb3$ = datatablesPluginBootstrap3( dt$ ); I tried to disambiguate all the names to eliminate confusion.. but in production code, I'd just overwrite var $ = require('jquery');
$ = require('datatables')($); // now with $.fn.dataTable
$ = require('datatables-plugin-bootstrap3')($); // now with internally modified $.fn.dataTable In the module datatables-plugin-bootstrap3 we would be access and expanding on @DataTables Please tell me if you'd be interested in accepting patches to |
Per the last note. Here are the two patches.
Now, just Please provide feedback... |
Hi, Thanks so much for looking into this - very much appreciated! I can see I'm going to have to spend some time getting to grips with this (I already knew that, but I'm drowning in support requests at the moment and time isn't easy to come by :-)). I absolutely see your point about the |
I'm not sure what you mean could be wrong. We're doing it the same way as the NPM blog precisely because it's right. You just have to get back to what a perfect module system would be,
In this case, you'd want
And viola.. that's the purpose of the build system. Delegate some of that work. Start to make some Roadmaps. You've got some really big development points I've raised. Plot out which ones you want to tackle and the priorities. There are other idioms that are also very common with CommonJS.. Some people prefer something like
or such.. in that case, you're just returning an Obj that has |
Okay - thanks for clearing that up for me - I understand now. The one thing I'm not seeing if the change from |
Well, I would not want people accessing the DataTables object except by configuration through jQuery or by proxy of jQuery because that's kind of weird. Lol, but shy of that, it's just a matter of style. I wouldn't think this was a good idiom except to confuse,
That, for me, feels like a violation of encapsulation. In the other way, you've got to actually expose api that shows you're mucking with jQuery internals.
or, whatever the syntax is you're on Further, that |
One use I've seen this use for is easily setting defaults and accessing the static API methods of DataTables - for example:
It makes sense to me to load a module and have it return that modules object directly I think. I've been trying to look into want some other libraries do such as Bootstrap and Foundation, but it seems to be added and removed from the libraries - Bootstrap for example is missing such support at the moment as best I can tell while they figure out some issues. |
First, as far as being biased, I'm against the god-object pattern and I'd suggest not making this globally configurable like that at all. 0-state is my preference. Why? If two people use the same jQuery they'll get two difference tables. That's very bad. I think these should only be options in the call to
Or even,
Again, for the reasons above.. You're breaking encapsulation if you're having your users rely on |
But this isn't an internal object structure, it is part of the documented public API. The ability to set defaults is something that has proven to be quite popular to be able to set those values site wide. It is also used by the Bootstrap integration among others. If you don't want to set defaults, no need to do so :-). Just pass the configuration options as you normally would to the DataTables "constructor". If I've made a design error there, it won't be the first and no doubt won't (unfortunately) be the last.
No doubt I'm being thick, but why would they get different tables? They are using the same jQuery and the same DataTables, so they'd get the same table based on the configuration supplied. |
You're missing the point. I don't want anyone setting my defaults either. Or, publishing a module up to npm that sets my default. See, your whole module is based on the idea of baking itself into jQuery. This is fine, and you rely on all the jQuery stuff being vanilla and no one having mucked with jQuery. Sane expectation. JQuery is popular because it doesn't lend itself to that kind of manipulation and sites can share one copy of it. But, you're telling your users to behave differently!! This means any app that shares jQuery and uses DataTables can inadvertently set defaults that impact my code. That's because you're storing state. This is bad practice generally, but it's definitely bad practice when you're publishing your module. The right way to do this is to wrap the constructor.... Or, to accept a jQuery object and write your own. // invoke as $("table").myAppDataTable({options})
$.fn.myAppDataTable = function (options) {
if ( options === undefined ) options[paging] = false
this.DataTable(options);
} Or, to write it as something else entirely.. // invoke as myAppDataTable( $("table"), {options} );
function myAppDataTable ( jQueryNode, options ) {
if ( options === undefined ) options[paging] = false
jQueryNode.DataTable( options );
} Anyway, you're going to cause a world of problems as CommonJS takes over the world and people use complex build systems. ;) Just expect it. |
I understand and see the issue now - thanks for the clarification. Its going to require a bit more thought on my part. |
Sure, and just fyi. This is in NPM's worst pratices. I'm happy to help out if you want to draft a roadmap to fix these issues. Just don't defend them with "backwards compatibility." You've got a great system but you'll need to update (and rather quick within the next couple of years), if you want to maintain dominance in this arena. CommonJS and build systems and npm-integration are just too big to neglect. |
Agreed - and I already have for too long. Let me have a bit of a think and get back to you on this - I need to understand a little bit more about what |
Sure, or on drawing up a staged roadmap to move forward with. Just give me a shout. |
Just want to add that some people use NPM instead of Bower to cherry-pick their packages and use something like Gulp with that. So just NPM already solves a lot of use cases (like browserify/webpack). I see that repos like https://github.com/DataTables/responsive (and bootstrap) are not in NPM and that's why I'm manually using the website downloader until everything goes stable. Anyway, DataTables is so amazing that I don't mind, but yeah it would be nice to have the option. Btw the NPM official DataTable package isn't automatically updated, maybe it could be automated. I'm sure there's something available for GitHub. https://www.npmjs.com/package/datatables "1.10.8 is the latest of 5 releases" |
@DataTables I'd love to tackle this or help tackle it. It's been over two months any progress? |
It's nearing the top of my todo list - support requests have been overwhelming recently. I'll be figuring out exactly what is required soon. |
Asking for help helps ;) On Tue, Sep 22, 2015, 22:36 Allan Jardine [email protected] wrote:
|
Pick any forum question and fire away :-). For this I want to understand the requirements of the packaging managers more. I don't really use them myself and there are quite a number available. My first step are deciding which to support across all of the DataTables software. Npm and bower most likely although I know composer will also be requested. |
There are basics you have to start now. I've already went over those.
The rest of this is easy.. It's just what .json files you want to add to support the specific repos. Bower is dead. Really all you need is a valid You're making this out to be something much harder than it needs to be. |
I'm sure I am but I need to be confident that I understand it and I currently am not - I haven't had time to look into it in detail since we last talked for a number of reasons. As I say, answering forum questions would be a massive help. Question - would you expect each api plugin and sorting plugin, language plugin etc to have their own umd wrapper. It seems like a lot of boiler plate code. Each in their own repo with their own npm package? I guess they probably should be. |
Yes, and their own repository. Why, because you can use them all independently of each other and you need to support all three module systems. |
That's around 160 repos that would need to be created. While obviously doable and scriptable, it seems rather messy to my mind. I wonder if an approach more like MomentJS's would be appropriate. Regardless, before I look in detail at how to split this repo up and out, I want to focus my attention on getting npm / require correct in DataTables core and its extensions as that is still lacking and I feel more pressing. |
Regarding composer, don't. fxp-composer-asset-plugin is a much used plugin for composer that adds support for installing bower and npm assets. That way at least you don't have to worry about that one. I don't care if I include a few extra MBs of files, if I don't need them I just don't reference them in my HTML code.. Also there is no real downside in doing this incrementally. |
Here is a bit of a brain dump: Distribution reposCurrently the repos contain:
Each of these repos will need 4*distribution repos (currently 4 styling options are supported - this will increase in future with the addition of Semantic UI). These are the repos that will be published on NPM and Bower (possibly NuGet as well although I might come back to that) - yes I do plan to support bower for now - it should be little additional work. They will be synced with the source repos using web hooks and build scripts on the DataTables.net server. ExtensionsCurrently the AMD / RequireJSCurrently DataTables defines itself as a named object, which is handy for loading the extensions, but it makes the configuration of Require a lot more rigid. Indeed the RequireJS documentation says it should be avoided if possible. I plan to make it an anonymous module as the expense of requiring that DataTables be included in the document before any of the extensions. CommonJS / NodeBased on your comments above, @EvanCarroll, I'm planning to commit this to DataTables core for its AMD loader handling: module.exports = function ($) {
// Get jQuery if it wasn't passed in
return factory( $ || require('jquery') );
}; The plug-ins are perhaps a little more interesting - for example the Bootstrap integration it would need some slightly more complex code such as: module.exports = function ($) {
if ( ! $ ) {
$ = require('jquery');
}
if ( ! $.fn.dataTable ) {
require('datatables')($);
}
factory( $ );
}; Extensions and their styling get a bit more messy. Open questions
I'd welcome your thoughts everyone! |
I feel like we're talking around each other. I've went over what needs to be done on multiple occasions and I'm unsure of the hold up. None of your ideas here are an improvement over what I suggested. We're just wasting lots of time. I have a production need for this code to complete. I'm considering forking DataTables and the Plugins I need. You can pull, but I'm going to have to publish to NPM. |
Hi. The hold up is simple time constraints on my part. I am working on this though and plan to publish the distribution repos that I talked about in the next two weeks - sooner if at all possible. |
New: CommonJS will load jQuery if it wasn't passed in Fix: Bootstrap, Foundation and jQuery UI integration Javascript files use module.exports correctly Dev: Change the file include "function" name to not conflict with `require` - AMD / RequireJS - The Require documentation strongly discorages using a named module, but I've used this in the past as the plug-ins need a name to depend upon themselves. This is still `datatables` but if the developer is using Require and it resolves automatically to a different name (which it may depending upon their configuration) they can use a map option to map their name to `datatables`. See moment/moment#1095 - CommonJS - Based on the disscussion in DataTables/Plugins#199 it seems that some developers like to pass a certain version of jQuery in. This modification allows them to do so while retaining backwards compatiblity. - Integration files - The UMD wrapper for these files have been restructured to be easier to follow. Also, based on the discussion in the Plugins issue noted above you can now pass in a jQuery instance or not and likewise a DataTables object or not. - To avoid direct conflict with `require()` the build scripts have been updated to use a "function" called `_buildInclude`. Ultimatily this should really be updated to use grunt or similar.
@EvanCarroll - Would you be willing to say how you are including the CommonJS stuff in the browser? I'm trying to go through the install process myself and this appears to be non-obvious. DataTables isn't even close to working in a windowless environment (without something like jsdom) and this npm post seems to suggest there isn't a single good option yet (although it is from Nov 2014 so it might of changed). |
CJS in the browser is if you're using something like browserify. Webpack also supports it, but webpack understands requirejs as well, so no real issue |
Super thanks! browserify is what I was just starting to look at for CommonJS. There are a few naming conflicts with existing packages unfortunately, so that might take a little while to get sorted out, or I'll use non-standard names for the conflicting packages, which would be unfortunate, but so it goes. Making progress to this though :-). |
New: CommonJS will load jQuery if it wasn't passed in Fix: Bootstrap, Foundation and jQuery UI integration Javascript files use module.exports correctly Dev: Change the file include "function" name to not conflict with `require` - AMD / RequireJS - The Require documentation strongly discorages using a named module, but I've used this in the past as the plug-ins need a name to depend upon themselves. This is still `datatables` but if the developer is using Require and it resolves automatically to a different name (which it may depending upon their configuration) they can use a map option to map their name to `datatables`. See moment/moment#1095 - CommonJS - Based on the disscussion in DataTables/Plugins#199 it seems that some developers like to pass a certain version of jQuery in. This modification allows them to do so while retaining backwards compatiblity. - Integration files - The UMD wrapper for these files have been restructured to be easier to follow. Also, based on the discussion in the Plugins issue noted above you can now pass in a jQuery instance or not and likewise a DataTables object or not. - To avoid direct conflict with `require()` the build scripts have been updated to use a "function" called `_buildInclude`. Ultimatily this should really be updated to use grunt or similar.
Here is the new UMD code. https://github.com/EvanCarroll/umd This is the wrapper DataTables should be using (I just wrote it and now I'm going to fight to get it upstream). This would make the calling convention
This will work under AMD, CommonJS (Node/PhantomJS/Browserify, etc), and Browser-globals. This is pretty close to my original suggest except we're supply |
The only aspect that is holding me up on releasing new versions is the RequireJS aspect. Anyone with expertise in RequireJS helping out will be showered with flowers (or something like that). Good point about the window object - although I don't see why DataTables would ever be used without a global window (and likewise global document), it isn't a utility library like MomentJS or Underscore, it fundamentally needs a display output. |
Really.. That's easy. Let's say you want to server side rendering of DataTables. With Node you can do that. // pseudo-code
var jsdomWin = require('jsdom');
jsdomWin.env( html, function (err, window) {
var $ = require('datatables')(window);
// ...
console.log( window.document.documentElement.outerHTML )
} ); etc.. RequireJS should just work if you're using the UMD template provided above. |
@DataTables I wouldn't ask that question on your forum. You should ask it on StackOverflow. RequireJS is on its way out. It's slow and convoluted, and inside the United States it's pretty much already lost steam. Anyway, what you want to do in your case is implement UMD (for the millionth time)...
So then in datatables.net-bs (which uses UMD) you simply add that shit to the
|
Thanks - it is still something I would like to support at the moment though since other packaging manager use it. I've asked it a few places, I'll add SO. The JS aspect isn't an issue - its the CSS aspect that I would like to understand and be able to document. i've still to make the |
…ives DataTables the ability to be used in a headless environment (server-side rendering for example) - Factory builder redesigned to pass in window and document to the factory method, mandating a small update to the AMD and Browser loaders - Main change in is the CommonJS loader which can now optionally have a window object passed in - if it is not passed in `window` will be used (if this is the case in a CommonJS environment without a root object being passed in an error will occur). - DataTables caches a reference to the jQuery instance so the plug-ins can easily reference jQuery while retaining their current return of the module they define. This basically means that DataTables core will include jQuery for the plug-ins. - This does increase the core library size by ~160 bytes which is rather frustrating, but I think this is the correct way to go about it - With thanks to Evan Carroll for input on this: DataTables/Plugins#199
…ives DataTables the ability to be used in a headless environment (server-side rendering for example) - Factory builder redesigned to pass in window and document to the factory method, mandating a small update to the AMD and Browser loaders - Main change in is the CommonJS loader which can now optionally have a window object passed in - if it is not passed in `window` will be used (if this is the case in a CommonJS environment without a root object being passed in an error will occur). - DataTables caches a reference to the jQuery instance so the plug-ins can easily reference jQuery while retaining their current return of the module they define. This basically means that DataTables core will include jQuery for the plug-ins. - This does increase the core library size by ~160 bytes which is rather frustrating, but I think this is the correct way to go about it - With thanks to Evan Carroll for input on this: DataTables/Plugins#199
…ives DataTables the ability to be used in a headless environment (server-side rendering for example) - Factory builder redesigned to pass in window and document to the factory method, mandating a small update to the AMD and Browser loaders - Main change in is the CommonJS loader which can now optionally have a window object passed in - if it is not passed in `window` will be used (if this is the case in a CommonJS environment without a root object being passed in an error will occur). - DataTables caches a reference to the jQuery instance so the plug-ins can easily reference jQuery while retaining their current return of the module they define. This basically means that DataTables core will include jQuery for the plug-ins. - This does increase the core library size by ~160 bytes which is rather frustrating, but I think this is the correct way to go about it - With thanks to Evan Carroll for input on this: DataTables/Plugins#199 Sync to source repo @5ea57765c075b407d0c795404b63f54efc3c000f
…ives DataTables the ability to be used in a headless environment (server-side rendering for example) - Factory builder redesigned to pass in window and document to the factory method, mandating a small update to the AMD and Browser loaders - Main change in is the CommonJS loader which can now optionally have a window object passed in - if it is not passed in `window` will be used (if this is the case in a CommonJS environment without a root object being passed in an error will occur). - DataTables caches a reference to the jQuery instance so the plug-ins can easily reference jQuery while retaining their current return of the module they define. This basically means that DataTables core will include jQuery for the plug-ins. - This does increase the core library size by ~160 bytes which is rather frustrating, but I think this is the correct way to go about it - With thanks to Evan Carroll for input on this: DataTables/Plugins#199 Sync to source repo @5ea57765c075b407d0c795404b63f54efc3c000f
…ives DataTables the ability to be used in a headless environment (server-side rendering for example) - Factory builder redesigned to pass in window and document to the factory method, mandating a small update to the AMD and Browser loaders - Main change in is the CommonJS loader which can now optionally have a window object passed in - if it is not passed in `window` will be used (if this is the case in a CommonJS environment without a root object being passed in an error will occur). - DataTables caches a reference to the jQuery instance so the plug-ins can easily reference jQuery while retaining their current return of the module they define. This basically means that DataTables core will include jQuery for the plug-ins. - This does increase the core library size by ~160 bytes which is rather frustrating, but I think this is the correct way to go about it - With thanks to Evan Carroll for input on this: DataTables/Plugins#199 Sync to source repo @5ea57765c075b407d0c795404b63f54efc3c000f
…ives DataTables the ability to be used in a headless environment (server-side rendering for example) - Factory builder redesigned to pass in window and document to the factory method, mandating a small update to the AMD and Browser loaders - Main change in is the CommonJS loader which can now optionally have a window object passed in - if it is not passed in `window` will be used (if this is the case in a CommonJS environment without a root object being passed in an error will occur). - DataTables caches a reference to the jQuery instance so the plug-ins can easily reference jQuery while retaining their current return of the module they define. This basically means that DataTables core will include jQuery for the plug-ins. - This does increase the core library size by ~160 bytes which is rather frustrating, but I think this is the correct way to go about it - With thanks to Evan Carroll for input on this: DataTables/Plugins#199 Sync to source repo @5ea57765c075b407d0c795404b63f54efc3c000f
…ives DataTables the ability to be used in a headless environment (server-side rendering for example) - Factory builder redesigned to pass in window and document to the factory method, mandating a small update to the AMD and Browser loaders - Main change in is the CommonJS loader which can now optionally have a window object passed in - if it is not passed in `window` will be used (if this is the case in a CommonJS environment without a root object being passed in an error will occur). - DataTables caches a reference to the jQuery instance so the plug-ins can easily reference jQuery while retaining their current return of the module they define. This basically means that DataTables core will include jQuery for the plug-ins. - This does increase the core library size by ~160 bytes which is rather frustrating, but I think this is the correct way to go about it - With thanks to Evan Carroll for input on this: DataTables/Plugins#199 Sync to source repo @5ea57765c075b407d0c795404b63f54efc3c000f
Man, this would really be nice. Right now we must require a bower dependency just to get datatables-plugins. We could use the shim mentioned above (drmonty-datatables-plugins), but an official package would be preferable, obviously. Same for datatables-bootstrap3-plugin (given @EvanCarroll's comment). |
@DataTables, I'm not positive (since I haven't read all of the comments above), but you might be able to close this one as well... This was a highly popular request! That being said, could you please clarify the following before closing? @EvanCarroll has an interesting comment regarding |
Yes - the The integration files in this repo will be removed with the next major revision. I've only kept them in place since others might have been using them before. I found it was unmaintainable to keep them here though - the integration for each extension was suffering, so the integration files are now in each source repo for the extensions (and DataTables core). The npm packages for DataTables now all have a core Javascript file and then an optional styling package. The DataTables npm page is the best way to see what to use. |
Great, thanks so much for the very quick replies and helpful info, I really appreciate it. |
Currently there is a shim repo at https://github.com/drmonty/datatables-plugins
As far as I can tell it would be relatively painless to move the package description files to the main repo (and update them where necessary).
The text was updated successfully, but these errors were encountered: