Skip to content
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

Support for UMD #33

Closed
jzaefferer opened this issue Sep 24, 2015 · 8 comments
Closed

Support for UMD #33

jzaefferer opened this issue Sep 24, 2015 · 8 comments

Comments

@jzaefferer
Copy link

@jzaefferer jzaefferer commented Sep 24, 2015

I tried building jQuery UI, which uses UMD without CJS, with browserify and this module. While its likely that I've missed something, it doesn't seem to work, and the "// TODO: Implement support for amdWeb UMD modules." comment implies that it can't work.

My test is here: https://github.com/jzaefferer/browserify-jquery-ui

Running npm run build will build a bundle, but it doesn't make a difference if deamdify is used or not, it won't do anything.

What it should be doing is to find the if ( typeof define === "function" && define.amd ) { check and resolve the define call inside of that.

@tbranyen
Copy link
Collaborator

@tbranyen tbranyen commented Sep 24, 2015

We merged 283a160 which claims to add it

@jzaefferer
Copy link
Author

@jzaefferer jzaefferer commented Sep 24, 2015

No release since April? Okay.

I installed via npm install jaredhanson/deamdify#master and ran my build script again, nothing changed. Eventually I found that there are two ways to apply transforms, -t only applies to "local" modules, -g applies to everything. With that, the autocomplete.js dependency is actually modified, which looks alright. Unfortunately it fails to include the jQuery dependency, instead it replaces it with this:

},{}],3:[function(require,module,exports){
module.exports = function () {
    return jQuery;
}();
},{}]},{},[1]);

Any ideas?

@StreetStrider
Copy link

@StreetStrider StreetStrider commented Jan 21, 2016

I have the very same issue.
jQuery UI for some reason dropped support for CJS in 1.11. So I've recalled deamdify.
jQ UI module has the following structure:

(function( factory ) {
        if ( typeof define === "function" && define.amd ) {

                // AMD. Register as an anonymous module.
                define([
                        "jquery",
                        "./core",
                        "./widget"
                ], factory );
        } else {

                // Browser globals
                factory( jQuery );
        }
}(function( $ ) {

return $.widget( "ui.tabs", {
        version: "1.11.4",
// …

So it is an AMD module with fallback to global (factory(jQuery)) with NO CJS (so it is not actually an full-UMD). It also has internal deps ('./widget') and external ('jquery').
deamdify transformation does not really do anything special with it, so this module works as «global-module».
I expect deamdify to pass define into such module and both internal and external deps work ok. Not happening. For now I'm using browserify-shim which treat jQ UI as global-ish and force me to manually resolve jQ UI dependencies, which is makin me sad.

@au-phiware
Copy link
Contributor

@au-phiware au-phiware commented Jul 21, 2016

jquery/sizzle has the same problem. I.e. (1) the call to define is guarded by the existence of define (that 283a160 does appear to overcome); and (2) the function passed to define relies on a variable from its enclosing closure, this means that deamdify only output line 2233 of sizzle.js. This seems like a major flaw.

au-phiware pushed a commit to au-phiware/deamdify that referenced this issue Jul 21, 2016
As mentioned in jaredhanson#33, popular libs often do their main work outside of
the factory. Thus the factory is reduced to return a variable that is
declared and defined in the enclosing closure. UMD boilerplate is
usually an afterthought so to preserve the code outside of the factory
is very important!
au-phiware pushed a commit to au-phiware/deamdify that referenced this issue Jul 21, 2016
@au-phiware
Copy link
Contributor

@au-phiware au-phiware commented Jul 21, 2016

I have created a sample project that uses my changes: https://github.com/au-phiware/deamdify-sample/

@tbrandvik
Copy link

@tbrandvik tbrandvik commented Jul 30, 2016

Just a quick user note in case somebody finds it useful: I have found that if I use the au-phiware deamdify module and add

"browserify": {
"transform": "deamdify"
},

to jquery-ui/package.json

then all the jquery-ui files are transformed correctly. Using "browserify -g deamdify ..." from the command-line didn't work for me as some of the third-party modules I use don't work with deamdify.

Thanks for fixing the deamdify bug and putting it up!

@au-phiware
Copy link
Contributor

@au-phiware au-phiware commented Jul 30, 2016

@tbrandvik, which third-party modules that you use don't work with deamdify?

@ishaikovsky
Copy link

@ishaikovsky ishaikovsky commented Aug 17, 2016

Last released version of underscore - is not getting correctly transformed for me as well using current deamdify code.

@tbranyen tbranyen closed this in 49e2508 Feb 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.