AMD support

[rxaviers]

http://wiki.jqueryui.com/w/page/66995889/AMD-Support

• Remove build from Grunt
• Remove "globals" from all files
• Script to rename all files
• Actually rename all files, removing the "jquery.ui." prefix, before landing this PR
• Copy updated i18n files to datepicker demos

After merging to master:

• Expose $.Widget.
• Split core into multiple files: http://bugs.jqueryui.com/ticket/9647 and http://bugs.jqueryui.com/ticket/9648
• Bring back support for testing minified files in unit tests. Needs to use actual build output.
• Use an AMD loader for tests.

Skip

• Handle CSS dependencies - comments-based as in jQuery Mobile, see Handle CSS dependencies from JS comments download.jqueryui.com#178

[rxaviers]

Some "global" variables need to be renamed, so they don't clash in the final bundle jquery-ui.js (because, each file UMD wrapper gets removed for optimization).

Shall we rename them case-by-case like the above, or shall we be more programmatic and follow some kinda pattern through all the files that prevents them from clashing? Eg. by using key-value pairs objects:

var core = {
  uuid: 0,
  ...
};

[scottgonzalez]

Neither. The variables shouldn't be global. We'll need to just address these on a case-by-case basis.

[rxaviers]

Ok, all the current clashing ones have been renamed here 13be9a0

[jzaefferer]

Scott, do you want those globals to be addressed as part of this PR? Or handle them later?

[rxaviers]

We're mimicking the build process of modernizr, which uses requirejs to merge the parts into the bundle; plus it processes the files, so each individual AMD wrapper (in our case UMD) is removed.

We need to address those globals here, or we'll have a problematic bundle js. :(

[tjvantoll]

Hey @rxaviers. I gave this branch a shot and only noticed one thing:

require([ "./jquery.ui.autocomplete" ], function( autocomplete ) {
    autocomplete === undefined; // true
});

Seems like the autocomplete widget should be available here.

The only other thing I noticed is that the...

 * Depends:
 *  jquery.ui.core.js
 *  jquery.ui.widget.js
 *  jquery.ui.position.js
 *  jquery.ui.menu.js

comments in the code are redundant now. Should we remove them?

[rxaviers]

Seems like the autocomplete widget should be available here.

It shouldn't, because autocomplete or any other component doesn't return anything, they just extend $. You should do this:

require.config({
  paths: {
    jquery: "path/to/jquery",
    jqueryui: "path/to/jquery-ui"
  }
});

require([ "jquery", "jqueryui/jquery.ui.autocomplete" ], function( $ ) {
    $.fn.autocomplete === undefined; // should be false
});

Does it make sense?

[rxaviers]

comments in the code are redundant now. Should we remove them?

Indeed they are redundant. I vote to remove them 👍

[scottgonzalez]

Regarding the return value for the module, @arschmitz and I are already planning on making the widget factory return the generated constructor. Once we do that, we should be able to just have:

return $.widget( "ui.plugin", { ... } );

The dependency comments should go away.

[jrburke]

I meant to respond to some comments around named modules, but looks like they are now classified as "outdated". Just to confirm then:

Named modules (define() calls with the first argument a string for that module's ID) should be avoided. If all of these modules are in a directory, say called 'jqueryui', then if that directory is either placed at the AMD loader's baseUrl, or if there is a paths config set up for 'jqueryui' then it should all work out. Consumers of these modules would use IDs like 'jqueryui/jquery.ui.autocomplete' to refer to these modules.

[rxaviers]

@jrburke,

jquery-ui depends on jquery. When defining the components, we do define(["jquery", ...]). So, it turns out we're supposed to let users know they should define the following in their config to use jquery-ui with AMD.

require.config({
  paths: {
    jquery: "path/to/jquery"
  }
});

Unless they have a jquery.js file with that exact name on his baseUrl. Right? (double checking)

[jrburke]

@rxaviers that is correct, either a paths config for 'jquery' or 'jquery.js' in the baseUrl.

There are fancier options like map config, but for single file JS dependencies, just paths config or ideally just dropping jquery.js in the baseUrl should be the encouraged routes.

[gfranko]

@jrburke Here is the link for the other discussions: #1029 (comment)

[rxaviers]

@jrburke Excellent! Thanks.

It would be awesome if you could bless these two files: (so, I am safe this change is good)

• ui/jquery.ui.autocomplete.js
• ui/i18n/jquery.ui.datepicker-pt-BR.js

PS: basically, I'd like you to check the define and possibly the UMD wrapper for jQuery plugins.

[jrburke]

@rxaviers those files look fine to me.

[rxaviers]

thanks!

[tjvantoll]

Regarding the return value for the module, @arschmitz and I are already planning on making the widget factory return the generated constructor. Once we do that, we should be able to just have:

return $.widget( "ui.plugin", { ... } );

@scottgonzalez Excellent. I would expect (and want) that.

[jzaefferer]

Making $.widget return the constructor is pretty easy to implement, just return the constructor: https://gist.github.com/jzaefferer/7b9236a9b9fabb9d3aff (also welcome to the tautology club)

Might as well implement that as part of this PR.

[jzaefferer]

At the meeting yesterday we discussed that the build task here is just for size comparisons, since everything else moved to DB. It looks like a lot of this PR would be unnecessary without that build task. Not that the work has been a waste of time, but the actual building needs to be implemented in DB, not here, right?

Maybe we should land that cleanup and the widget-returns-constructor change before this PR, then rebase.

[jzaefferer]

Why does the regex literally contain "mysterious semicolon"?

[rxaviers]

Makes sure I remove the right thing.. But, the idea is to figure out why this is happening and fix the root cause :P

[scottgonzalez]

Please don't make the return value part of this PR. It will need to happen much sooner than this is going to land. I will land it when @arschmitz has finished a proof of concept in the Mobile repo.

[scottgonzalez]

When the rest of the discussion settles down, we need to talk about tracking CSS dependencies. But I don't want to discuss that until there are fewer other discussions going on.

[rxaviers]

At the meeting yesterday we discussed that the build task here is just for size comparisons, since everything else moved to DB. It looks like a lot of this PR would be unnecessary without that build task. Not that the work has been a waste of time, but the actual building needs to be implemented in DB, not here, right?
Maybe we should land that cleanup and the widget-returns-constructor change before this PR, then rebase.

Right, the build would be "moved" to DB. We'd only land the new UMD wrapped source files (knowing they work). Will let you guys know when it's updated.

[rxaviers]

The biggest cons of removing the build task is that we are not able to jshint the final output on each commit anymore. Therefore, it's hard to know whether variables among components might clash.

[rxaviers]

DownloadBuilder is ready to build these UMD files jquery/download.jqueryui.com#156

[rxaviers]

By the way, thanks for the review and fixes.

[rxaviers]

Rebased.

PS: Just figured out why git didn't revolve the rename automatically. Because, it's not a simple rename, it's a rename plus content changes (from @import url("jquery.ui.<widget>.css"); to @import url("<widget>.css");)

[tjvantoll]

Ah, that makes sense.

[scottgonzalez]

We really need to expose $.Widget in addition to $.widget. I'm ok with waiting until this lands in master though, since I want to split some other files as well.

[rxaviers]

Split core into multiple files.

Is this going to happen before or after 1.11?

[scottgonzalez]

Split core into multiple files.

Is this going to happen before or after 1.11?

I'd like to do it before 1.11.

[rxaviers]

I'd like to do it before 1.11.

Ok, thanks. I will add one issue to handle that on DB.

[rxaviers]

Changes, per @scottgonzalez review, complete.