Release: Cutoff duplicate parts with download builder

[rxaviers]

Fixes jquery/download.jqueryui.com#122.

Related changes:

• DownloadBuilder PR: Export DownloadBuilder, so jQueryUI Release Script can use it download.jqueryui.com#139

[rxaviers]

@scottgonzalez @jzaefferer raise your thumbs, suggestions, comments

[jzaefferer]

I haven't reviewed the DB changes yet, just glanced at them. Though looking at the changes on this side, I wonder why there are so few deletions. This is supposed to reduce duplication, but we mostly have duplicated more code without really removing anything.

You mention that you didn't change the release_cdn task. That seems to be what's prevented deleting more code. So why keep that? jquery/download.jqueryui.com#122 specifically listed all three release tasks to be moved.

In addition to that I think this should be taken a step further. There's no reason for these build_* tasks to be inside grunt. They should be moved to the release script, preferabbly with less overhead then there's now. One piece, the checking if downloadbuilder is installed, can then go away, since the release task already takes care of installing it.

[rxaviers]

I haven't reviewed the DB changes yet, just glanced at them. Though looking at the changes on this side, I wonder why there are so few deletions. This is supposed to reduce duplication, but we mostly have duplicated more code without really removing anything.

I also wondered we'd have less code in here. Part of that is because the release package is not exactly the same thing of the download package. Having less code in here has the counter effect of moving more code into DownloadBuilder that wont be used there. It's totally possible. If that's the will, just let me know.

You mention that you didn't change the release_cdn task. That seems to be what's prevented deleting more code. So why keep that? jquery/download.jqueryui.com#122 specifically listed all three release tasks to be moved.

Certainly. Although, I don't see how we can use current DownloadBuilder to accomplish that. Unless we move the build tasks into DownloadBuilder as well.

In addition to that I think this should be taken a step further. There's no reason for these build_* tasks to be inside grunt. They should be moved to the release script, preferabbly with less overhead then there's now. One piece, the checking if downloadbuilder is installed, can then go away, since the release task already takes care of installing it.

Well, they are inside of grunt simply because they were there before :). But, I agree with you. Let's move them. I guess the reason they were there was because the convenience of re-using some grunt tasks. The build_* tasks have very fewer bindings to grunt. But, we still do use some stuff from grunt, eg. configs. If you are ok with the following, let's first define the above items, then we go further into this step.

[rxaviers]

Team, request for your comments. See below.

We have two clear different entities:

1. jQueryUI: set of widgets, interactions, and effects (js and css) for the browser.
2. jQueryUI DownloadBuilder/ThemeRoller: web tool that allows users to customize jQueryUI (js and css) for development or production usage, which requires them no tech understanding of the build process and no software dependency to execute it (the build process).

We have another not so clear one, kinda split in half in two different places, which is the:
3) jQueryUI build tool (or tasks, or process, whatever name). It's a set of procedures that:

• Concatenate the source files (js and css) handling its dependencies, and minify for production. This is partially in jQueryUI, partially in DB [i];

• Generate manifest files, and generate the authors file. Completely in jQueryUI [ii];

• Arrange the files on different directory structures and pack them. This is partially in jQueryUI, partially in DB [iii];

• Customize the build with different component selection (js) and/or different theme vars (css). Completely in DB [iv];

  Little bit of more details:
  i) On jQueryUI, we have the build and sizer* grunt tasks that concatenate and minify the source files, along with some other things. On DB, we have the same thing when assembling the custom bundles using different code.
  ii) jQueryUI generates the manifest jsons and the md5 manifest for the release package. The AUTHORS file generation lives in the release script.
  iii) On jQueryUI, we have the code to arrange the release package structure. On DB, we have the code to arrange the downloaded package structure.
  iv) On DB, we have all the code that allows custom builds.

Problems (that we all know):
a) Different results. We have two code in two different places producing the "same" result. Therefore, we are subject to divergences. Current ones: different minified files, and different banners (some differences have been fixed by this PR, but some like the order of which the component files appear are still). The current differences may be acceptable, but it doesn't guarantee we're safe from potential buggy ones in the future.
b) Two maintenance. Whenever a css style changes (jquery/download.jqueryui.com@8c36ea9), or whenever a banner license changes (jquery/download.jqueryui.com@b067fd8), or whenever we use a different uglify version (or a parameter), or whenever we have a new file (or one less) (jquery/download.jqueryui.com@a89634c), etc, we need to change two codes instead of one.

Possible solution:
One that addresses both problems above:
a) Build consistent results;
b) Require one only maintenance;
Plus:
c) Do not require a bunch of if version a do this, if version b do that. Whenever we start adding that much complexity to the code (like we current have in DB), one thing is true, we're doing it the wrong way. One thing is evident, the "build tool" is very tied together to the "jQueryUI".
d) Allow a user not to depend on the web tool DB. It should be a facilitator, not a requirement. We have users that cannot build his custom jQueryUI anymore, because we don't live support the legacy version he's using. A user should be able to build a custom bundle (custom component selection js and/or custom theme vars css) using his own source files in the command line too.

So, a possible solution:

• Make DownloadBuilder AND jQueryUI to use the same "build tool".
• Host the "build tool" inside jQueryUI. (or a separate npm project, but it would require a very tight version correlation with jQueryUI).
• jQueryUI grunt tasks: build, sizer_, release_ to use this new "build tool" API to execute such tasks.
• DownloadBuilder to rely on this new "build tool" API to custom build the packages. (and still be able to cache things the way we are doing. As far as I saw, we are able to).
• The new "build tool" API is the low level download builder. Most of it, we already have from the current DownloadBuilder. We can obviously modify them and re-define together a new set of methods if we want to.

I think of something really abstract and simple to use like:

theme = new Theme(vars);

build = new Build(components, theme);
build.sizes(); // get bundle sizes
build.writeTo( path ); // create a path with the build files
build.zipTo( path ); // create a zip file with the build files

release = new Release();
release.writeTo( path ); // create a path with the pre-release files
release.zipTo( zipPath ); // create a zip file with the pre-release files

release = new Release({cdn: true});
release.writeTo( path ); // create a path with the release-to-cdn files
release.zipTo( zipPath ); // create a zip file with the release-to-cdn files

That's it.

[scottgonzalez]

ii) jQueryUI generates the manifest jsons and the md5 manifest for the release package. The AUTHORS file generation lives in the release script.

The AUTHORS file generation is not part of the release script. The release script does a pre-release check to verify that the AUTHORS file is up-to-date.

a) Different results. We have two code in two different places producing the "same" result. Therefore, we are subject to divergences. Current ones: different minified files, and different banners (some differences have been fixed by this PR, but some like the order of which the component files appear are still). The current differences may be acceptable, but it doesn't guarantee we're safe from potential buggy ones in the future.

For any actually released files, we should have a single code path. That's the goal of this.

b) Two maintenance. Whenever a css style changes (jquery/download.jqueryui.com@8c36ea9),

There's only one place for this to be updated. We never touch the tokens inside jquery-ui.

or whenever a banner license changes (jquery/download.jqueryui.com@b067fd8),

Maybe we can get away with not using banners inside jquery-ui. Official releases will always go through the full release process anyway.

or whenever we use a different uglify version (or a parameter),

We need to find a way to tie the settings to jquery-ui. The code repo needs to control how it gets processed so that building an old version works the same as it did when the version first came out.

or whenever we have a new file (or one less) (jquery/download.jqueryui.com@a89634c),

/externals/ should just be globbed. Theming is trickier.

c) Do not require a bunch of if version a do this, if version b do that. Whenever we start adding that much complexity to the code (like we current have in DB), one thing is true, we're doing it the wrong way. One thing is evident, the "build tool" is very tied together to the "jQueryUI".

What are the specific things we're doing based on version?

d) Allow a user not to depend on the web tool DB. It should be a facilitator, not a requirement. We have users that cannot build his custom jQueryUI anymore, because we don't live support the legacy version he's using. A user should be able to build a custom bundle (custom component selection js and/or custom theme vars css) using his own source files in the command line too.

This has always been the plan. Well, whether or not we build a CLI was never determined. But the download builder is supposed to be a simple node module and download.jqueryui.com is supposed to build on top of that. But we want all of that in a single repo.

Make DownloadBuilder AND jQueryUI to use the same "build tool".

Depending on what you mean by "build tool", that's what download builder is supposed to be.

Host the "build tool" inside jQueryUI. (or a separate npm project, but it would require a very tight version correlation with jQueryUI).

If this includes things like building themes, I'm not sure I want this in jquery-ui.

jQueryUI grunt tasks: build, sizer_, release_ to use this new "build tool" API to execute such tasks.

The release tasks are supposed to go away. We already discussed that. They belong inside release.js. The size task is tied to grunt. We can ditch that, but I'd like to know what the real benefit is. We can make the grunt build much less production-ready if we want to.

DownloadBuilder to rely on this new "build tool" API to custom build the packages. (and still be able to cache things the way we are doing. As far as I saw, we are able to).

It sounds like you're still asking for 3 repos. We don't want three. Two is plenty. Where does the benefit come from in having three repos?

The new "build tool" API is the low level download builder. Most of it, we already have from the current DownloadBuilder. We can obviously modify them and re-define together a new set of methods if we want to.

Why can't we just keep this inside the download.jqueryui.com repo?

The build task can probably just go away. I don't think we actually use it for anything, other than priming a release. @jzaefferer is there any reason to keep the build task in jquery-ui if we pull the rest of the release tasks out of grunt?

[rxaviers]

What are the hardest things for DownloadBuilder to figure out dynamically when building a download package (or a release package)? In other words, what is needed to make DB loosely coupled from jQueryUI (absent of build tools).

1. To build the download package we rely on these following hard coded data https://gist.github.com/rxaviers/5412659. So, the first step is to pass that metadata to DB. (perhaps along with manifests?)

solution: possibly, another manifest JSON file

2. The second step is to let DB know all its exception rules:

• Demo files https://github.com/jquery/download.jqueryui.com/blob/release/lib/builder.js#L282
• Doc files https://github.com/jquery/download.jqueryui.com/blob/release/lib/builder.js#L307
• Ad Hoc section https://github.com/jquery/download.jqueryui.com/blob/release/lib/builder.js#L357

solution: should not be hard to address

3. The third step is theme:

• Theme files https://github.com/jquery/download.jqueryui.com/blob/release/lib/builder.js#L319
  eg. The current code is very specific to pick/change specific files. This wont be trivial to pass via a metadata file (I've been thinking for example about the consideration to remove jquery.ui.base, how do we handle the pre and post eras without hard if versions).
• Theme vars. Although this doesn't happen often, I think it's hard to rely on that. Because, theme should be free to evolve too. So, ideally we'd come up with a dynamic solution here too.
  https://github.com/jquery/download.jqueryui.com/blob/release/lib/themeroller.js#L120

solution: seems addressable, but not trivial

4. The forth step, the most tightly coupled code (from my point of view) are the templates logic. So far we have a template for banner and another one for the demo index. Passing the template dynamically is easy. The hard is if we need downloadBuilder to pass any different parameter to the template for any reason. Or, if we need to make any other file template generated too.

solutions?

For a more complete view, I've assembled a summary of "Big picture of the communication (input and output) between the building blocks of jQueryUI DownloadBuilder".

[rxaviers]

For comparison, what is the hardest thing for DownloadBuilder to figure out dynamically when building a download package (or a release package) if we put the build tools into jQueryUI?

• None of the above of course. Because, all that JS logic will be tightly coupled with jQueryUI. So, whenever we change something in jQueryUI, we also do it in the build code and it's in the same repo.
• Change/addition of an input (other than https://gist.github.com/rxaviers/5412513#input) AND one such that requires changing the low level build tool of jQueryUI.
• Any other ones that I could not prevent right now... 👻

What are the downsides?

• Increase the size of the repo by ~196KB (which is current total size of DB/lib/* directory).

What are the Features of each one?

	jQueryUI (build absent )	jQueryUI (current build)	jQueryUI (smarter build)
Online build	✔	✔	✔
Online custom build	✝	✝	✝
Offline build	✘	✔	✔
Offline custom build	✘	✘	✔

• Online build: download the full package via DownloadBuilder or CDNs.
• Online custom build: download the custom package via DownloadBuilder.
• Offline build: grunt build.
• Offline custom build: grunt build:datapicker,accordion.

✝ Only during supported period.

[jzaefferer]

@rxaviers those last two comments sounds very much like the discussion we had earlier about abstracting DB into a generic tool. Unless I'm misunderstanding that, what value does that provide right now, where the goal is remove the duplication between UI and DB?

[rxaviers]

We de-duplicate it or moving things to UI, or DB, or a third-thing.
1:2 comment: List of things we need to move it into DB (as we discussed);
2:2 comment: List of things we need to move it into UI (comparing it with the 1:2 comment);
Please, just let me know if it's not clear.

[scottgonzalez]

1. Keep categories manifest inside download builder, since it's only used for UI configuration. Expose everything else through jquery-ui repo.

2.Demos: Add additional data to manifest files to list additional demos.
2.Docs: Parse the docs URL from the manifest file. Grab the path, remove the trailing slash, add ".html".
2.AdHoc:

• Datepicker: TODO
• Scale Effect: Use additional demos metadata from 2.Demos.

3.Files: I'm not sure why there needs to be any special handling here for the CSS files. Just copy every file, replace vars, inline imports, etc.
3.Vars: In the future, we should try to avoid reusing a variable name if the meaning changes. For this specific issue, I think we'll just leave the conditional for a while (until we decide to stop supporting 1.9).

1. I think we can just provide the templates in jquery-ui. The data they rely on shouldn't really change, and if it does, we can always write logic to make the template backward compatible.

[rxaviers]

2.Demos: Add additional data to manifest files to list additional demos.

Should I change it like this?

diff --git a/ui.effect.jquery.json b/ui.effect.jquery.json
index 839fa77..4e1f529 100644
--- a/ui.effect.jquery.json
+++ b/ui.effect.jquery.json
@@ -58,7 +58,17 @@
        ],
        "bugs": "http://bugs.jqueryui.com/",
        "homepage": "http://jqueryui.com/",
-       "demo": "http://jqueryui.com/effects/",
+       "demo": [
+               "http://jqueryui.com/effect/",
+               "http://jqueryui.com/addClass/",
+               "http://jqueryui.com/animate/",
+               "http://jqueryui.com/hide/",
+               "http://jqueryui.com/removeClass/",
+               "http://jqueryui.com/show/",
+               "http://jqueryui.com/switchClass/",
+               "http://jqueryui.com/toggle/",
+               "http://jqueryui.com/toggleClass/"
+       ],
        "docs": "http://api.jqueryui.com/category/effects-core/",

jqueryui.com currently have a page for each of the above. Is it already listed somewhere? How jqueryui.com figures that information out?

[jzaefferer]

I'm pretty sure Scott had something else in mind for that, but can't find it.

[scottgonzalez]

The demo field cannot be an array. The manifest files are for the plugins site, so we don't have free reign over them. The plan is to add an _ field which contains another hash for the additional data. I'd prefer if we can just have a function which download builder calls to get the raw data, so that we don't need to publish the _ field to the actual manifest files.

[rxaviers]

One naive question: What is demo field supposed to represent? Where to find demos for such file? It's interesting, because http://jqueryui.com/effects/ doesn't show what ui/jquery.ui.effect.js (core) really implements, only part of it. From that page, there's no link to all the other effects listed above.

About the function approach. Do you have an example?

[scottgonzalez]

What is demo field supposed to represent?

http://plugins.jquery.com/docs/package-manifest/#field-demo

About the function approach. Do you have an example?

No, it can be whatever we want it to be. I haven't had time to do anything with it.

[rxaviers]

http://plugins.jquery.com/docs/package-manifest/#field-demo

It says the demo field is: "The url to the plugin demo or demos". What we currently have for effects core isnt accurate then.

No, it can be whatever we want it to be. I haven't had time to do anything with it.

Ideas:

• Change jqueryUi's package.json file and add a main key-pair value to point to a exports file. Place that function in there? So, we can (from DB) require(path/to/jquery).<that function>??
• Add an _extra field with another demo in there for effects (core): _extra: {demo: [ ... ]}?
• Move all effects (core) demos into effect/ subdir?

Which sounds better for you? Any other one?

[rxaviers]

We have the very same analogous situation with docs of effect-scale, its json manifest file lists "docs": "http://api.jqueryui.com/scale-effect/". Although, there are two more documentation urls it should list: puff-effect, and size-effect.

Scott, what's the implication of optionally using an Array on those fields (demo and docs)? We don't break backward compatibility with existing plugins. We are able to check if those fields are a String or an Array and use them appropriately (at least DB is already doing this).

[rxaviers]

Guys, I wait for your feedback on the above to keep going...

[rxaviers]

When running the release script in "dry-run mode", I was wasting too much time to download a github repo all the time. So, I've added the above to allow a filesystem repo to be used.

This change is in a separate commit to make it easier to be removed if wanted.

[scottgonzalez]

This is fine, but can you amend the commit to also add a note in the usage section at the top?

[rxaviers]

added

[scottgonzalez]

Same as the comment above about spacing and phrasing.

[rxaviers]

Reviews fixed

[jzaefferer]

remove the path var here, it gets defined below

[rxaviers]

I've landed things on DB... Do you want to do a final test by running the release script before we land this one too?

curl https://raw.github.com/jquery/jquery-ui/8f664481438fc906a829cbfe4daf6403831e2754/build/release/release.js

[jzaefferer]

The dist_units_images and cdn copy targets still exists - are these still needed?

[rxaviers]

Well remembered. There should be no cdn reference in Gruntfile anymore. So, deleted.

About dist_units_images, it is still used in the task below. So, keeping it.

grunt.registerTask( "build", [ "concat", "uglify", "cssmin", "copy:dist_units_images" ] );

[jzaefferer]

Ah, right, we use that to run unit tests with minified sources. Deserves a line comment.

Otherwise looking good.

How do I test this? The curl call is one thing, but how do I run it to make sure it actually builds the right thing (without actually releasing anything)?

[rxaviers]

PS: for the record

<rxaviers> jzaefferer, `copy:dist_units_images` (from build task) is used by unit tests? Shall we add `build` as part of `test` task then? = grunt.registerTask( "test", [ "build", "qunit" ] )?
<rxaviers> jzaefferer, I've `grep "dist" -R` on tests path, the only thing I see is:
 includeStyle( "dist/jquery-ui.min.css" );
 includeScript( "dist/jquery-ui.min.js" );
 by tests/unit/testsuite.js
 anyway, I just want to double check with you dist usage by tests
<jzaefferer> yeah, but that css file has relative images urls
<rxaviers> so, it's appropriate to add build to test isnt it?
<jzaefferer> no, `grunt test` never needs those images
<rxaviers> it's only a visual thing?
 what about  includeScript( "dist/jquery-ui.min.js" );
<jzaefferer> only runs when QUnit.urlParams.min is true
 not happening for `grunt test`
<rxaviers> ok

[rxaviers]

@jzaefferer aside from curl to fetch the script, one should be able to test the script the same way as before (http://wiki.jqueryui.com/w/page/59346425/Release%20Checklist). Note that, now, a local repo (eg. --remote /tmp/jquery-ui) could be used (for me, that makes it quicker than using a gh account for the dry-runs).

Three things that needs double checking are:

• __release/repo output. If it still produces the same thing as before - this should not have changed;
• the __release/*zip package. If the content is the expected;
• *versions are calculated correctly;

[jzaefferer]

I'm trying to test the updated release script. To have something to compare to, I'm trying to run the release script from master. For that I had to modify the script, since I can't figure out the prevVersion: https://gist.github.com/rxaviers/5850493

With that in place, it fails at the generate_themes task:

Running "generate_themes" task
Warning: object is not a function Use --force to continue.

Stacktrace for that:

Warning: object is not a function Use --force to continue.
TypeError: object is not a function
    at Object.<anonymous> (/Users/jza/dev/ui-release-test-ref/__release/repo/build/tasks/build.js:184:47)
    at Object.thisTask.fn (/Users/jza/dev/ui-release-testref/__release/repo/node_modules/grunt/lib/grunt/task.js:78:16)
    at Object.<anonymous> (/Users/jza/dev/ui-release-test-ref/__release/repo/node_modules/grunt/lib/util/task.js:282:30)
    at Task.runTaskFn (/Users/jza/dev/ui-release-test-ref/__release/repo/node_modules/grunt/lib/util/task.js:235:24)
    at Task.<anonymous> (/Users/jza/dev/ui-release-test-ref/__release/repo/node_modules/grunt/lib/util/task.js:281:12)
    at Task.start (/Users/jza/dev/ui-release-test-ref/__release/repo/node_modules/grunt/lib/util/task.js:290:5)
    at Object.grunt.tasks (/Users/jza/dev/ui-release-test-ref/__release/repo/node_modules/grunt/lib/grunt.js:155:8)
    at Object.module.exports [as cli] (/Users/jza/dev/ui-release-test-ref/__release/repo/node_modules/grunt/lib/grunt/cli.js:38:9)
    at ChildProcess.<anonymous> (/usr/local/lib/node_modules/grunt-cli/bin/grunt:44:22)
    at ChildProcess.EventEmitter.emit (events.js:98:17)

[jzaefferer]

Managed to get the release working by changing the script to install DB@1.10.3-3 (instead of -4).

I've got the result in two repos, one for the updated script: https://github.com/jzaefferer/jquery-ui
One for the current from master: https://github.com/jzaefferer/jquery-ui-tmp

Along with the two commits, I compared the content of "jquery-ui-1.11.0-cdn.zip". The new release script doesn't create a dist folder anymore, so that's the only zip file I can compare directly. I did a treediff, to check if there's any structural differences (nope) and compared a few random files by looking at the header, to see if there's any difference there (nope). I tried to do a file diff between the two directories, but couldn't figure out to do that properly.

I'm not super confident on the thoroughness of my testing, but I think this is good to go.

[rxaviers]

We definitely need a better dependency control inside the release script. Currently, we install the latest version of each dependencies. So, it's subject to get broken. :P

[jzaefferer]

Well, make a start and add it as part of this PR?

[rxaviers]

Added. Although, note that previous versions of this script won't be fixed. Because, they use grunt tasks which, in turn, required users to install download.jqueryui.com manually.

[rxaviers]

Merged by a52421b ece91a5 7321df7 4ed52cb