Declare test dependencies using AMD

[rupl]

With #486 taken care of we have individual files for all core detects. Now we have to declare dependencies. @SlexAxton suggested AMD and there seems to be consensus on that. He suggested cheating a lil' bit by assigning results directly to Modernizr global.

@paulirish asked Alex "for your AMD proposal, why wouldnt we return inside the modules and pull in the value as the dep?" And he said that's so we can ship the built version without any wrappers at all (almond or otherwise).

Couple examples:

define('canvastext', ['canvas'], function () {
  var ret;
  if (Modernizr.canvas){
    ret = document.createElement('canvas').getContext('2d').fillText == 'function';
  }
  Modernizr.canvastext = !!ret;
});

or CJS-style

define(function (require) {
  require('canvas');

  var ret;
  if (Modernizr.canvas){
    ret = document.createElement('canvas').getContext('2d').fillText == 'function';
  }
  Modernizr.addTest('canvastext', ret);
});

[theodoreb]

I'm not sure it's relevant to go with AMD if it's done like that. Pinged @jrburke to get his opinion.

(My concerns are around hardcoded name and implicit globals, I'll let him have at it though).

[SlexAxton]

Its relevant because we can run a build through r.js and it'll do the right thing
We don't anticipate people using each test as a separate module.

[SlexAxton]

More specifically, now that I'm not mobile:

On a normal build, even Almond will nearly double the size of the code. We'd also need namespacing on our require system, etc, so it didn't conflict with other people's. Then if we wanted you to be able to require tests via modules, we'd need to optionally turn off that namespacing code.

My ideal is that Modernizr registers (optionally, perhaps) as an AMD module and the whole object is returned. However, I think we can use the great dependency managed build system internally in order to generate a small build without having to do builds the way we have been in the past (writing everything as a file and removing the tests with regex).

Now, we can run r.js and then remove the wrappers around each module, know that they are in the correct order, and add the define call as an optional feature.

[SlexAxton]

(My concerns are around hardcoded name and implicit globals, I'll let him have at it though).

We can avoid both of these things, really. If we have matching filenames to our property names:

define(['modernizr', 'window', 'document', 'addTest'], function (Modernizr, window, document /*, dont bring in the rest */) {
   Modernizr.addTest('filename', function (){
     return !!document.filename || !!window.filename;
  });
});

But it wouldn't actually cause any issues if we assumed those three things to be implicit globals.

[dmethvin]

Having not used r.js I am wondering how it handles local variables inside the closure. Are they hoisted to the top of the combined file? How does it deal with name conflicts?

[SlexAxton]

r.js wouldn't be doing anything to the code except reading it's dependencies and putting the full files in order.

At that point, I'd go in and strip the wrappers (probably with an IIFE re: name conflicts). Then I'd run uglify on them.

Does that seem reasonable? (I've been considering the same thing for the modular jQuery pseudo AMD thing, since you had requested no module wrappers left over)

[dmethvin]

Ah, so r.js leaves the IIFE, that makes sense given that it wants to preserve the modularity. And stripping the wrappers for this case seems like the right thing to do, given how small most of the code will be inside the wrapper. Basically, the AMD require is just a declarative dependency header but it seems like a good syntax to use so you can piggyback on the tools that are available.

For jQuery core we just came up with a naming convention for globals so that variables could be shared across "modules" after the IIFEs are removed. It's not elegant but it works.

[SlexAxton]

Right. I'm essentially using AMD to use the awesome toolchain for building. But we don't want to expose those modules or require their cruft to the average user. (Though it would be easy to make a build that did just that).

[jrburke]

I was invoked by @theodoreb, but I believe @SlexAxton has the right approach if using r.js for the builds.

I'm sure @SlexAxton has enough context, but using the onBuildWrite hook in the build config for r.js would allow you to use a regexp to do the define() stripping/transform.

Feel free to ping me if you want me to look anything over, need pointers on how to get it done.

[theodoreb]

Thanks for stopping by! If it ends up possible to have AMD definitions we can use in a bigger AMD project I'm fine with whatever really :) What is so good about Modernizr are the tests, not so much the global Modernizr variable :)

What I had in mind was something like :

define(['Modernizr', 'Modernizr/canvas'], function (Modernizr, canvas) {
  return Modernizr.canvastext = !!(canvas && is(/* rest of the test */));
});

Which looks very much like Modernizr.addTest();, and I can actually use this as-is in an AMD project. I know that r.js can prefix define, which could end up after build as ModernizrLoader.define(). Since the dependency work has be done before, and the tests are based on browser capabilities not configuration (no need for config support of the AMD spec) there wouldn't be a need to use almond.js.

And if the modules can be defined as define('Modernizr/canvastext', [], function () {}); implementing define/require would be dead simple without worrying about namespaces confilcts.

Then the init of the lib can end up as:

require(['Modernizr', 'Modernizr/canvastext'], function (Modernizr) {
  window.Modernizr = Modernizr;
});

I don't know where is comes from in the define or if there are tests that won't fit well in there but that's the idea.

I mean not trying to get in the way, just want to provide a possible alternative. I know you guys have awesome build tools with grunt and all so that's probably not much of a concern for you the way it looks like in the source. I prefer writing source code than build code, so that'd be how I'd use AMD for it :p

[SlexAxton]

I totally feel you on most of this, but unfortunately, we are the minority.

What is so good about Modernizr are the tests, not so much the global Modernizr variable :)

Modernizr didn't invent feature testing. It just put a bunch of them into one big object and added some css classes to html and that's what a lot of less technical people love about it.

require(['Modernizr', 'Modernizr/canvastext'], function (Modernizr) {

This is not a bad idea, but unfortunately, for many uses of modernizr, this won't work. We need to actually run all of the modernizr modules synchronously at load time, because they get added to the html class as a bulk operation before any of the markup is rendered. If we wanted css classes to show up of tests we didn't use in javascript, we'd have to just require them in order to get them to execute, and then not use them.

Not to mention, people would need to require modernizr tests from our Modernizr.require unless we somehow registered all of our modules in their build based on some condition (possible, but harder).

In other words, we kind of have to build and pre-run each test. I'm not against eventually creating a no-css lazy evaluation mode, but we've got to maintain some semblance of backwards compatibility.

Since the dependency work has be done before, and the tests are based on browser capabilities not configuration (no need for config support of the AMD spec) there wouldn't be a need to use almond.js.

That's what almond.js is for, if I'm not mistaken. We can namespace Modernizr.define/require but that is where almond would live. It still has to maintain a list of the registered modules and stuff. I'm sure we could take out some features, but that seems like some work.

It seems like it'd be easier to do it the cheating way, and convert it to the lazy eval all AMD module way. We'd just need to add a return Modernizr['filename'] to the end of each module and it'd all work.

[theodoreb]

almond.js does kinda much more than needed here. it takes care of the config object, resolve paths and all.

AMD doesn't have to be Async, it's the definition that is, not necessarily the loader. Code 100% not tested:

MdzL.reg = {};
MdzL.resolve = function (dep) {
  return MdzL.reg[dep];
}
MdzL.define = function (id, deps, fac) {
  var resolvedDeps = deps.map(MdzL.resolve);
  MdzL.reg[id] = fac.apply(this, resolvedDeps);
};


MdzL.define('Mdz', function () { return {};});
MdzL.define('Mdz/canvas', ['Mdz'], function (Mdz) {});
MdzL.define('Mdz/canvastext', ['Mdz', 'Mdz/canvas'], function (Mdz, canvas) {});

And the require part is just a way to make r.js pick up the right modules to include.

Obviously it needs a check for the number of arguments and some safeguards in some spots, still should work pretty well considering the tests are a TRUE/FALSE type of results. The execution would be sync since we don't have to load anything and r.js made sure it's in the proper order.

[SlexAxton]

You are correct that we could reimplement our own AMD runtime. What say you about the filesize argument?

[theodoreb]

That depends on how much dependencies you have between the tests. When there are no dependencies it'd go from

tests['canvas'] = function() {};

to

Modernizr.define('M/canvas', ['M'], function (M) {});

and dependencies would give

Modernizr.define('M/canvastext', ['M', 'M/canvas'], function (M, C) {});

I tried on the current tests (copy/pasted all the tests[] without the rest), as you said, gzip takes care of most of it. It's very crude testing so take the number as-is: I have 30b of diff over a 1792b of minified+gziped JS so around 1.7% of filesize increase with the define+deps.

If there are a lot of dependencies it's different. Can you give me an idea of how much inter-dependencies there are?

But I assume that a build script can take care of stripping most if not all of the overhead off, It'd make it simpler to use what is needed from the outside though. It all comes down to the priorities I guess. Since I'm from Drupal and that's kinda how we'd like to use Modernizr, I'm obviously biaised :D

[SlexAxton]

You guys are looking into using Modernizr as AMD for each of the individual tests? That's great.

I'd love to figure out something that does this. I think you're on to something with the auto-running define calls. There is definitely some environment stuff to work out, though (colliding or registering in other environements).

If we have a define that auto-runs the tests, so we can do the html class stuff, how do we integrate into someone's app? Do we dual register? Once in Modernizr.define, and once in if(window.define && window.define.amd)?

[theodoreb]

Well I don't know about the other Drupal guys but I am :) Why do you think @rupl and @seutje worked on the branch to split everything up? Because we'd need it for Drupal. And actually, now it's using Modernizr.addTest() instead of tests[] so It's even closer to the define thing :) rupl tells me that there is hardly any inter-dependencies so the filesize would actually increase by less than 1.7% after their branch is merged.

As for the define issue, to me there are two very different use cases:

1. the standalone build. What is currently existing. This would be using the Modernizr.define() calls and a very simple amd loader similar to what my code does above would be included with it. As far as users are concerned, no change from what is happening now.

2. the Drupal-like use-case where the system is looking to integrate modernizr into it's workflow. AMD would be the best choice (modernizr on nodejs anyone? :p) for that.The Modernizr source code (which would be using straight up define, r.js takes care of prefixing them if needed) is integrated into the system, and used like any other AMD module, goes through r.js, uglifyjs, what have you. After the build almond.js or similar can be used to declare require/define and use the modules. If you have a require call straight away the execution will be immediate. Same as current behavior.
   Now if people want to lazy-load tests that'd be possible also, they'd be using requirejs or alike to do that. Only in this case the non-immediate execution could be an issue with html classes and all.

So no it wouldn't dual register. A third party would be using the AMD modules, nothing special. It'd be his resposibility to minify/concatenate (even name?) the modules in his application. if he can't do that he should just stick with the standalone build anyway.

[jrburke]

I apologize because I'm not up to speed on all things Modernizr/Drupal, but this is my understanding how the use cases @theodoreb outlined would work. I think this is just restating what @theodoreb is expressing above:

The Modernizr tests use define() with their dependencies in source form, then the Modernizr build process can use r.js to combine all the modules together, and then convert the define([], function (){}); to something like (function(){}()); and all the tests use Modernizr.whatever inside them to reference their dependencies, and the output of the Modernizr build should like similar to how it does today.

If someone else wanted to use the Modernizr tests with a r.js build config that does not translate the define() calls to IIFEs, then that should work fine too, as long as the define() calls inside the tests use relative module IDs. That other build config could then either decide to bundle almond, use the r.js namespacing, whatever it likes, even its own define() implementation that immediately executes the define() factory functions.

I would probably expect most folks to just use the Modernizr build config though, given the needs of some of the tests (delayed execution would not be desired), and that there normally should not be a need for two different Modernizrs on a page.

[theodoreb]

Let me know if you need more data or examples or anything. I'd be happy to help.

FYI, on the Drupal side, having Modernizr in Drupal 8 core happens before 1st of december or it doesn't happen. I appreciate that you have your own schedule and I'm not trying to rush things. Modernizr would be very helpful to us and I'd like to see it happens. It'd be too bad if we miss the opportunity because of a few weeks :)

[SlexAxton]

I don't think it's a matter of how long it takes to program it. It'll probably be one or two nights of getting core stuff in place, and maybe a few more nights to get the builder working again.

I think this discussion shows, though, that we don't know exactly how we want to do everything. I'll dive into some code with this feedback and see if I can't come up with something that everyone's happy with. I think I agree with what everybody wants. I don't know if we're all on the same page implementation-wise, but perhaps that just means the first implementation should shed enough light to either keep or fix.

I'll keep you updated.

[SlexAxton]

https://github.com/slexaxton/Modernizr/tree/mod-mod

This is a full AMD-ification of everything in Modernizr.

This still needs require.js to function, so now I'll need to write the code that strips out the wrappers.

If everyone could just serve the base directory (npm install serve then serve . from the base directory) from that branch, and go to http://127.0.0.1:3000/modular.html you can play around with all the different modules.

[SlexAxton]

If you want to build, just run ./build.sh and you'll end up with a dist folder. I have a comment in the modular.html file that will load the built version instead of the unbuilt version, if you want to see that.

[ryanseddon]

Wow nice work.

I've grabbed your branch and converted the build.sh file into a grunt task so we can just do grunt build so it'll work cross platform.

[paulirish]

this is huge.

super nice work, alex.

On Wed, Nov 7, 2012 at 8:47 PM, Ryan Seddon notifications@github.comwrote:

Wow nice work.

I've grabbed your branch and converted the build.sh file into a grunt task
so we can just do grunt build so it'll work cross platform.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/713#issuecomment-10176797.

[seutje]

Thanks a bunch, Alex, you even split up the core helper functions, awesome!
Is there anything left you'd like some help with? So I can return the favor and have you wake up to such awesomeness...

[SlexAxton]

I added a few things and cleaned up a few of the tests in the process of porting them over.

I made the Modernizr object a Modernizr instance (new Modernizr()) and I add all non-tests to the prototype. So when you console.dir(Modernizr) you only get tests in the core object, and you could potentially do a hasOwnProperty over the object to get only tests back. You can get at the rest of the functions in the __proto__ nest.

I added addAsyncTest which is pretty much just to keep the test running in the execution queue in the right spot. Inside of a addAsyncTest you'd call addTest again, once you have the result.

The addTest of anything that is built with the new AMD stuff is different than the one we expose. Since we want to have a single operation where we switch out class names, we queue up all the tests and their results and then after they've all run, we inject into the head. Async tests run in the event loop, but they don't call addTest until after it's been replaced with the user facing one. This is so they can individually update the html classes since that has already occured. This means that all addTest calls inside of an addAsyncTest must be async, which means in some cases, you might need to do a setTimeout(..., 0) to force that to happen. Otherwise it will get added to the queue of things to run, after they've already run.

I added an options-like object as the third parameter of addTest (just in the internal mode, for now). The only thing in it is an aliases property that takes an array of aliases for that test. The core name and all of it's aliases are then set on the modernizr object. It should be a good way to do the next thing:

We need to standardize the naming conventions of the tests. They're all over the place. But in the name of backwards compat, we should probably keep around the old names for a version. You can put them in the aliases. A few of the tests already have this, if you need an example. Can someone else take charge on this? I think it'll be really important to get right. I know @paulirish has some good feedback on leaving out hyphens and stuff.

I'm getting close to successfully removing the AMD wrappers and @jrburke is a badass and said he could get in a browser capable version of r.js in soon, so we're in good shape to have this stuff nailed down.

I'm sure I have more, but this is good for now.

[SlexAxton]

Oooh. I think I liked about the setTimeout thing. As long as you pull in addTest as a module, and don't just use the one on the Modernizr object (better for munging anyways), you'll get the external function. So you don't have to jump through that hoop.

[SlexAxton]

Quick update:

The mod-mod branch now successfully strips all AMD wrappers and outputs a dev version (with comments) and minified version of Modernizr, along with gzip numbers and stuff.

So this is now minimally viable. I just need to work on the input format for choosing tests and stuff, since this just always builds everything.

I'm not great with grunt or shell scripting so if someone wants to clean up what I did, that'd be great.

Note: The file size output only works on os x, I couldn't find a good cross platform way of outputting file size in bash. maybe just switch that part to node :/

[SlexAxton]

Another good task for someone would be to verify that I didn't break any of the tests. Can someone compare the output of the new modernizr and the old modernizr and make sure it all matches up?

[SlexAxton]

Another idea if someone wants to take it:

There is some danger that we blow away local variables when we strip AMD wrappers. I made sure that every module only has one or zero top level local variables, but in the future people might not realize that. So it'd be good to run a minimal jshint over a stripped version during a build and check for 'this variable has already been declared' warnings. Or something similar. Then we can catch these situations before they cause problems.

[ryanseddon]

@SlexAxton you should move this branch into Modernizr and call it AMD or something so we can all work on it.

I've got grunt doing all the work on my machine that your build.sh script was doing so I'll add that in when you move it over.

[SlexAxton]

I put my mod-mod branch as the 3pre branch on modernizr origin

https://github.com/Modernizr/Modernizr/tree/3pre

So I'll work out of that from now.

[ryanseddon]

I've added the build step to the grunt file ea721fb

I also added a package.json which will be the source of truth for the version, license and project name. Also has reference to some dependencies, grunt and a grunt-contrib plugin so you'll need to run npm install.

Once you've got all that the default task for grunt will do the full build.

[SlexAxton]

Sorry for the Australian delay. I think I have all the necessary pieces in place, just needs some clean up at this point.

If you check out the 3pre branch (on the main fork) at the root directory, you can:

A) run ./bin/modernizr config-all.json

This could probably be ported to something in grunt, but I figured that was someone else's area of expertise. Right now it's a 4 line shell script that calls into some modules. If you mess with these, make sure you don't break the browser compat.

The config structure is pretty simple now. We will probably want to add a spot to do class prefixes and stuff, but this is a good start.

B) serve the root directory. Open up modular.html

This will show 3 textareas. The first is a big configuration object that you can edit. I default it to everything just for stress testing, but feel free to change it.

Then when you press the button, it will run the config through the same code as on the server, but via the clientside r.js stuff that @jrburke graciously released for us. The only unforgivable hack is that I'm generating the root include file, and thus it does not exist on the file system. For now I hardcoded the 'response' of where it tries to request to return the contents of a global variable. @jrburke if there's anyway to take a string that is a file (of a require call, specifically) and add it without actually requesting for it, I'd love to know how. :D

Once it requests and builds, etc, it injects the 'dev' version into the top box, then uses the instance of Uglify2 (yay free upgrade) that's on the requirejs object, and minifies the code and injects it into the second box. To test, just copy the contents and paste it into your console. It should update your classes, and you can log out the Modernizr object after that as well.

It's quite late, so I'm sure I'm forgetting bunches, but this should get us over the hump. It's all a bit messy though, so all you guys/gals who have an eye for clean code should jump in.

Still needed: We need to decide on official names and categorization asap.

[jrburke]

@SlexAxton I committed a change in r.js master (will be part of an eventual 2.1.3 release, but no ETA for it yet) where you can seed text content for given module IDs via a rawText build config option:

https://github.com/jrburke/r.js/blob/master/build/example.build.js#L486

You can try it out by using the latest master build of r.js:

https://raw.github.com/jrburke/r.js/master/dist/r.js

The change is tracked in requirejs/r.js#324 if you need to give feedback on the rawText feature.

[SlexAxton]

Alright, I think I have things working pretty well.

https://travis-ci.org/Modernizr/Modernizr/builds/3508819

All tests pass in my browsers and in phantom/travis.

grunt build is currently hooked up to always just build config-all which should be everything. So the grunt stuff should be closer, but if someone smarter can hook up the command line arguments to take an optional json file as input instead would be welcomed.

There are no tests with hyphens in them right now, so we may not have the best organized tests, but at least they all are easy to access, and are backwards compat. If people want to organize them better, I think it's a good idea, but I'm not sure it should hold up a release.

My next steps are to more or less hook up the 3.0 code to the current /download page (with the necessary modifications).

[jokeyrhyme]

Given addAsyncTest, does this work also resolve #622 ?

[patrickkettner]

This seems complete to me, @SlexAxton do you agree?

[SlexAxton]

I think so!

[patrickkettner]

beauty, thanks dude