Skip to content

Export manifest logic #1015

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions build/core.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,22 @@
"description": "The core of jQuery UI, required for all interactions and widgets.",
"homepage": "http://jqueryui.com/",
"demo": "http://jqueryui.com/",
"docs": "http://api.jqueryui.com/category/ui-core/"
"docs": "http://api.jqueryui.com/category/ui-core/",
"_": {
"docs": [
"http://api.jqueryui.com/data-selector/",
"http://api.jqueryui.com/disableSelection/",
"http://api.jqueryui.com/enableSelection/",
"http://api.jqueryui.com/focus/",
"http://api.jqueryui.com/focusable-selector/",
"http://api.jqueryui.com/jQuery.ui.keyCode/",
"http://api.jqueryui.com/removeUniqueId/",
"http://api.jqueryui.com/scrollParent/",
"http://api.jqueryui.com/tabbable-selector/",
"http://api.jqueryui.com/uniqueId/",
"http://api.jqueryui.com/zIndex/"
]
}
},
"datepicker": {
"description": "Displays a calendar from an input or inline for selecting dates.",
Expand Down Expand Up @@ -31,7 +46,20 @@
"category": "effect",
"homepage": "http://jqueryui.com/",
"demo": "http://jqueryui.com/effect/",
"docs": "http://api.jqueryui.com/category/effects-core/"
"docs": "http://api.jqueryui.com/category/effects-core/",
"_": {
"docs": [
"http://api.jqueryui.com/addClass/",
"http://api.jqueryui.com/color-animation/",
"http://api.jqueryui.com/effect/",
"http://api.jqueryui.com/hide/",
"http://api.jqueryui.com/removeClass/",
"http://api.jqueryui.com/show/",
"http://api.jqueryui.com/switchClass/",
"http://api.jqueryui.com/toggle/",
"http://api.jqueryui.com/toggleClass/"
]
}
},
"position": {
"description": "Positions elements relative to other elements.",
Expand Down
3 changes: 3 additions & 0 deletions build/main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
manifest: require( "./manifest" )
};
94 changes: 94 additions & 0 deletions build/manifest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
var categories, flatten, manifest, pkg;

categories = {
core: {
name: "ui.{plugin}",
title: "jQuery UI {Plugin}"
},
widget: {
name: "ui.{plugin}",
title: "jQuery UI {Plugin}",
dependencies: [ "core", "widget" ]
},
interaction: {
name: "ui.{plugin}",
title: "jQuery UI {Plugin}",
dependencies: [ "core", "widget", "mouse" ]
},
effect: {
name: "ui.effect-{plugin}",
title: "jQuery UI {Plugin} Effect",
keywords: [ "effect", "show", "hide" ],
homepage: "http://jqueryui.com/effect/",
demo: "http://jqueryui.com/effect/",
docs: "http://api.jqueryui.com/{plugin}-effect/",
dependencies: [ "effect" ]
}
};

flatten = function( flat, arr ) {
return flat.concat( arr );
};

pkg = require( "../package.json" );

manifest = function( options ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't want an API that takes options. We have exactly two use cases and they are more clearly served with two functions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any name suggestions? I need your creativity help here ;)

Manifest: {
regular: fn // that grunt task will use
withExtras: fn // that db will use
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{
    manifests: function() {}, // full merged data
    rawManifests: function() {} // raw data with _
}

The nicer named API is what is used externally.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. If we are going to have those two forms, it means:

  • grunt task will have to process the manifests (by removing the _ from rawManifests);
  • we'll name manifests the invalid (full merged) manifests, which is only useful for download builder;

If we expose one only option (raw format), it will mean the same for the grunt task (it will have to process it anyway), and we move the merge logic (which is only useful for db) to db.

What do you think if we only export the raw manifests then?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The manifests are only invalid in terms of what the plugin registery expects. Our manifests can be more feature complete than that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't make sense to ask a 3rd party (which DB is) to process the _ fields.

options = options || {};

return Object.keys( categories ).map(function( category ) {
var baseManifest = categories[ category ],
plugins = require( "./" + category );

return Object.keys( plugins ).map(function( plugin ) {
var manifest,
data = plugins[ plugin ],
name = plugin.charAt( 0 ).toUpperCase() + plugin.substr( 1 );

function replace( str ) {
return str.replace( "{plugin}", plugin ).replace( "{Plugin}", name );
}

manifest = {
name: data.name || replace( baseManifest.name ),
title: data.title || replace( baseManifest.title ),
description: data.description,
keywords: [ "ui", plugin ]
.concat( baseManifest.keywords || [] )
.concat( data.keywords || [] ),
version: pkg.version,
author: pkg.author,
maintainers: pkg.maintainers,
licenses: pkg.licenses,
bugs: pkg.bugs,
homepage: data.homepage || replace( baseManifest.homepage ||
"http://jqueryui.com/{plugin}/" ),
demo: data.demo || replace( baseManifest.demo ||
"http://jqueryui.com/{plugin}/" ),
docs: data.docs || replace( baseManifest.docs ||
"http://api.jqueryui.com/{plugin}/" ),
download: "http://jqueryui.com/download/",
dependencies: {
jquery: ">=1.6"
},
// custom
category: data.category || category
};

if ( options.extra ) {
Object.keys( data._ || {} ).forEach(function( prop ) {
manifest[ prop ] = data._[ prop ];
});
}

(baseManifest.dependencies || [])
.concat(data.dependencies || [])
.forEach(function( dependency ) {
manifest.dependencies[ "ui." + dependency ] = pkg.version;
});

return manifest;
});
}).reduce( flatten, [] );
};

module.exports = manifest;
77 changes: 2 additions & 75 deletions build/tasks/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,81 +19,8 @@ function expandFiles( files ) {
}

grunt.registerTask( "manifest", "Generate jquery.json manifest files", function() {
var pkg = grunt.config( "pkg" ),
base = {
core: {
name: "ui.{plugin}",
title: "jQuery UI {Plugin}"
},
widget: {
name: "ui.{plugin}",
title: "jQuery UI {Plugin}",
dependencies: [ "core", "widget" ]
},
interaction: {
name: "ui.{plugin}",
title: "jQuery UI {Plugin}",
dependencies: [ "core", "widget", "mouse" ]
},
effect: {
name: "ui.effect-{plugin}",
title: "jQuery UI {Plugin} Effect",
keywords: [ "effect", "show", "hide" ],
homepage: "http://jqueryui.com/effect/",
demo: "http://jqueryui.com/effect/",
docs: "http://api.jqueryui.com/{plugin}-effect/",
dependencies: [ "effect" ]
}
};

Object.keys( base ).forEach(function( type ) {
var baseManifest = base[ type ],
plugins = grunt.file.readJSON( "build/" + type + ".json" );

Object.keys( plugins ).forEach(function( plugin ) {
var manifest,
data = plugins[ plugin ],
name = plugin.charAt( 0 ).toUpperCase() + plugin.substr( 1 );

function replace( str ) {
return str.replace( "{plugin}", plugin ).replace( "{Plugin}", name );
}

manifest = {
name: data.name || replace( baseManifest.name ),
title: data.title || replace( baseManifest.title ),
description: data.description,
keywords: [ "ui", plugin ]
.concat( baseManifest.keywords || [] )
.concat( data.keywords || [] ),
version: pkg.version,
author: pkg.author,
maintainers: pkg.maintainers,
licenses: pkg.licenses,
bugs: pkg.bugs,
homepage: data.homepage || replace( baseManifest.homepage ||
"http://jqueryui.com/{plugin}/" ),
demo: data.demo || replace( baseManifest.demo ||
"http://jqueryui.com/{plugin}/" ),
docs: data.docs || replace( baseManifest.docs ||
"http://api.jqueryui.com/{plugin}/" ),
download: "http://jqueryui.com/download/",
dependencies: {
jquery: ">=1.6"
},
// custom
category: data.category || type
};

(baseManifest.dependencies || [])
.concat(data.dependencies || [])
.forEach(function( dependency ) {
manifest.dependencies[ "ui." + dependency ] = pkg.version;
});

grunt.file.write( manifest.name + ".jquery.json",
JSON.stringify( manifest, null, "\t" ) + "\n" );
});
require( "../manifest" )().forEach(function( manifest ) {
grunt.file.write( manifest.name + ".jquery.json", JSON.stringify( manifest, null, "\t" ) + "\n" );
});
});

Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"name": "jQuery Foundation and other contributors",
"url": "https://github.com/jquery/jquery-ui/blob/master/AUTHORS.txt"
},
"main": "build/main.js",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although this is practical for our urgent need, this seems wrong.

Exporting API this way allows 3rd parties (eg download builder) to do the following:

manifest = require("jquery-ui").manifest;

This is practical, but isn't it wrong to have the main entry of jquery-ui not being the jquery-ui scripts? Eg. ui/jquery.ui.js

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps, we could have a custom-entry in packages.json, eg:

"manifest": "build/manifest.js"

Or DB could hijack it with require("./jquery-ui/build/manifest").

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need for main to point at a source file since the source files are worthless in node.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that's perhaps what a potential components.json's main entry will
be for. Nevermind.

"maintainers": [
{
"name": "Scott González",
Expand Down