Wrap enquire.js to allow its consumption as an AMD module. #61

tomdavies · 2013-06-10T10:21:00Z

We wanted to be able to use enquire.js internally in our all-AMD environment, so I wrote this little patch to conditionally wrap enquire as an AMD module using the UMDjs pattern

It currently uses grunt-rigger for in-place replacement of source files in the AMD version (https://github.com/buildjs/rigger / https://github.com/DamonOehlman/grunt-rigger) and outputs to dist/amd/. To be honest I only really used this to avoid touching the core source in order to make maintaining our fork easier.

The UMD template output works both as a global and as an AMD module, so if you were willing to accept a version that updated the default to use this pattern let me know and I'll submit a pull request that does that instead.

WickyNilliams · 2013-06-10T17:06:47Z

Thanks for this. I was actually thinking about this recently myself as I've just got into requirejs.

Questions:

What is the standard practice for libraries, to have two versions or one?
What is the file size difference between the AMD version and the regular version?
Is there some way we could use rigger to completely replace concat in the build? I'd feel happier knowing we had one approach for putting files together instead of two

I'll check this out later though, it's definitely needed

tomdavies · 2013-06-10T20:58:25Z

What is the standard practice for libraries, to have two versions or one?

One. Having looked at this a little more, the most common pattern (eg jQuery) is definitely to expose the library as a named module with something like this at the end of the lib:

global.enquire = global.enquire || new MediaQueryDispatch();

if ( typeof define === "function" && define.amd ) {
define( "enquire", [], function () { return global.enquire; } );
}

The advantage of the UMDjs pattern used in my pull request is that the returned module is anonymous, but named modules are considered more robust by some pretty smart people.

As I say my initial aim was to wrap enquire while preserving the easy of updating our fork, so my code may actually not be the best fit for rolling into the core lib.

What is the file size difference between the AMD version and the regular version?

Minified: 112 bytes or about 6%:

File "dist/enquire.min.js" created.
Original: 8836 bytes.
Minified: 1859 bytes.
Gzipped:  426 bytes.
File "dist/amd/enquire.min.js" created.
Original: 9014 bytes.
Minified: 1974 bytes.
Gzipped:  448 bytes.

Is there some way we could use rigger to completely replace concat in the build? I'd feel happier knowing we had one approach for putting files together instead of two

You could easily use rigger to replace concat, though concat is definitely the more standard/common approach.

Personally I like rigger's sprockets-like syntax for uses like this, as it enables you to keep the include where it belongs (in code itself) rather than in metadata (the Gruntfile). For concating modules we're using grunt-contrib-requirejs so don't really have need of concat and I can't really speak to its advantages/disadvantages.

If you just roll with the UMD version (or with a named module) you wouldn't actually need to generate two output files as I say, as the UMD pattern just checks for the existence of define() and if it doesn't find it just sets a global as per enquire's current functionality.

WickyNilliams · 2013-06-18T18:24:05Z

Excuse the delay in response, it's been a busy week.

Thanks for the detailed answers! I think based on what you've said I'd favour a named module. I guess the file size increase would be smaller if we went that route too?

As for rigger, I think I will embrace that approach over concat, I like the way it works. I tried it perhaps a year back for that reason, but couldn't get it to work at the time - so it's good to see it's all good now.

So the final question remains: would you like to amend your PR to have a named module (and squash commits ideally), or would you like me to bin it off and do it myself?

tomdavies · 2013-06-18T19:45:37Z

I'm happy to amend and squash, should get time over next couple of days.
—
Tom Davies

tom@tomdavies.net
07971851208

On Tue, Jun 18, 2013 at 7:25 PM, Nick Williams notifications@github.com
wrote:

Excuse the delay in response, it's been a busy week.
Thanks for the detailed answers! I think based on what you've said I'd favour a named module. I guess the file size increase would be smaller if we went that route too?
As for rigger, I think I will embrace that approach over concat, I like the way it works. I tried it perhaps a year back for that reason, but couldn't get it to work at the time - so it's good to see it's all good now.

So the final question remains: would you like to amend your PR to have a named module (and squash commits ideally), or would you like me to bin it off and do it myself?

Reply to this email directly or view it on GitHub:
#61 (comment)

WickyNilliams · 2013-06-18T20:17:08Z

Awesome! Nice one Tom
On 18 Jun 2013 20:45, "Tom Davies" notifications@github.com wrote:

I'm happy to amend and squash, should get time over next couple of days.
—
Tom Davies

tom@tomdavies.net
07971851208

On Tue, Jun 18, 2013 at 7:25 PM, Nick Williams notifications@github.com
wrote:

Excuse the delay in response, it's been a busy week.
Thanks for the detailed answers! I think based on what you've said I'd
favour a named module. I guess the file size increase would be smaller if
we went that route too?
As for rigger, I think I will embrace that approach over concat, I like
the way it works. I tried it perhaps a year back for that reason, but
couldn't get it to work at the time - so it's good to see it's all good
now.
So the final question remains: would you like to amend your PR to have a
named module (and squash commits ideally), or would you like me to bin it

off and do it myself?

Reply to this email directly or view it on GitHub:

#61 (comment)

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/61#issuecomment-19636103
.

WickyNilliams · 2013-07-04T18:55:43Z

hey @tomdavies did you get anywhere with this afterwards?

tomdavies · 2013-07-05T10:32:16Z

Hey, really sorry - I've been swamped on a day-job project, which is now
finished.

Thanks for the prod, I'll do it tonight

On 4 July 2013 19:55, Nick Williams notifications@github.com wrote:

hey @tomdavies https://github.com/tomdavies did you get anywhere with
this afterwards?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/61#issuecomment-20489907
.

Tom Davies
Digital Design & Development

0117 230 3238 | 07971 851 208
tomdavies.net | @metadaptive

WickyNilliams · 2013-07-05T16:13:52Z

No worries, I forget about stuff like this all the time so just thought I'd give you a nudge.

However I forbid you to do it tonight - it's FRIDAY!!! :)

mhrisse · 2013-07-15T14:42:37Z

+1

WickyNilliams · 2013-07-18T18:55:09Z

@tomdavies I've just switched the build to solely use rigger instead of concat task. Unless you've got a burning desire to do the named module bit, I'll probably do it tomorrow (despite my previous advice to not do stuff on a friday :)

@mhrisse soon!!

tomdavies · 2013-07-19T09:32:28Z

@WickyNilliams that fine mate - sorry for not getting to this - suddenly
insanely busy at the day job. On the plus side looks like we'll be using
enquire pretty heavily on chunky project, so thanks again for all your work

On 18 July 2013 19:55, Nick Williams notifications@github.com wrote:

@tomdavies https://github.com/tomdavies I've just switched the build to
use the rigger instead of concat task. Unless you've got a burning desire
to do the named module bit, I'll probably do it tomorrow (despite my
previous advice to not do stuff on a friday :)

@mhrisse https://github.com/mhrisse soon!!

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/61#issuecomment-21205964
.

Tom Davies
Digital Design & Development

0117 230 3238 | 07971 851 208
tomdavies.net | @metadaptive

WickyNilliams · 2013-07-22T19:09:30Z

Hey guys, I've pushed a first cut of UMD support in enquire to the following branch: https://github.com/WickyNilliams/enquire.js/tree/amd

Could someone please review it for me and check it works as intended? I've tested with a standard script tag and all is well, but I don't have any setups using require/other in order to test that end of things

WickyNilliams · 2013-07-22T19:09:59Z

PS. I'm going to close this since I'm pushing on with it myself, just as a housekeeping exercise

WickyNilliams · 2013-07-30T19:05:10Z

Could someone please check out what I've done and confirm it's good to ship? I've done some rudimentary testing with require, which seemed good. But I'm not au fait with it enough to know if anything is amiss.

mhrisse · 2013-07-31T07:08:15Z

Thanks! Will do asap.

tomdavies · 2013-07-31T09:32:48Z

Just dropped the new built JS into the moderately large AMD project that I originally wanted to use enquire with (and that prompted my original pull request) and all looks like it works fine. Haven't spent ages manually testing but my JS loads without error and all my tests still pass, and your UMD implementation look fine.

👍

The only slight issue for AMD users to perhaps be aware of (though it was true in my version too and I can't see a way around it anyway) is that enquire is still a global, even when available inside of a module definition. This means your queries are not private to/closed over by your module definition. They are available via the enquire that will exist in the global context as well, so calling enquire.unregister() without passing the name of a function to unregister, eg:

enquire.unregister("screen and (orientation: landscape)");

rather than:

enquire.unregister("screen and (orientation: landscape)", handlerToUnregister);

Will unbind all functions bound to "screen and (orientation: landscape)", not just those that have been bound within the current context.

I know that outside of an AMD usecase that's probably by design, and it's fine in AMD land too as long as you're expecting it to happen, you just have to be disciplined about being specific when unregistering your handlers.

Lastly, I'm probably going to submit another patch to allow for optionally passing an array of functions to unregister() to make cleaning up after oneself gracefully a little more easy, assuming that would be welcome?

mhrisse · 2013-08-21T13:48:08Z

This works on our large scale site just fine! Great, thanks a lot! However, I am currently wondering how to make this play nicely with modernizr "the AMD way". Any pointers appreciated.

WickyNilliams · 2013-08-21T20:28:51Z

Sweet, perhaps I'll merge this into master soon then, was just waiting for some people to confirm that it works well as it is.

I'm not familiar with require enough to be able to offer advice unfortunately. How do you usually deal with polyfills? Someone else must have tackled the AMD + Modernizr + polyfill problem before?

chiplay · 2013-11-06T17:48:41Z

@WickyNilliams @mhrisse We are having the same issue trying to polyfill enquire for older IE inside a require.js build. I'm doing this currently with a modification to the UMD signature of enquire - but not sure how to make this universal (since everyones path vars could be different for media-match:

;(function (name, context, factory) {
    var matchMedia = context.matchMedia;

    if (typeof module !== 'undefined' && module.exports) {
        module.exports = factory(matchMedia);
    }
    else if (typeof define === 'function' && define.amd) {
        if (!matchMedia){
            define(['media-match'],function() {
                var matchMedia = context.matchMedia;
                return (context[name] = factory(matchMedia));
            });
        } else {
            define(function() {
                return (context[name] = factory(matchMedia));
            });
        }
    }
    else {
        context[name] = factory(matchMedia);
    }
}('enquire', this, function (matchMedia) {

tomdavies added 2 commits June 10, 2013 11:14

Wrap enquire to allow its consumption as an AMD module.

6727afb

make amd version return enquire so that it actually works

6f98dd5

WickyNilliams closed this Jul 22, 2013

WickyNilliams mentioned this pull request Jan 7, 2014

do not pollute global namespace #97

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wrap enquire.js to allow its consumption as an AMD module. #61

Wrap enquire.js to allow its consumption as an AMD module. #61

tomdavies commented Jun 10, 2013

WickyNilliams commented Jun 10, 2013

tomdavies commented Jun 10, 2013

WickyNilliams commented Jun 18, 2013

tomdavies commented Jun 18, 2013

So the final question remains: would you like to amend your PR to have a named module (and squash commits ideally), or would you like me to bin it off and do it myself?

WickyNilliams commented Jun 18, 2013

off and do it myself?

WickyNilliams commented Jul 4, 2013

tomdavies commented Jul 5, 2013

WickyNilliams commented Jul 5, 2013

mhrisse commented Jul 15, 2013

WickyNilliams commented Jul 18, 2013

tomdavies commented Jul 19, 2013

WickyNilliams commented Jul 22, 2013

WickyNilliams commented Jul 22, 2013

WickyNilliams commented Jul 30, 2013

mhrisse commented Jul 31, 2013

tomdavies commented Jul 31, 2013

mhrisse commented Aug 21, 2013

WickyNilliams commented Aug 21, 2013

chiplay commented Nov 6, 2013

Wrap enquire.js to allow its consumption as an AMD module. #61

Wrap enquire.js to allow its consumption as an AMD module. #61

Conversation

tomdavies commented Jun 10, 2013

WickyNilliams commented Jun 10, 2013

tomdavies commented Jun 10, 2013

WickyNilliams commented Jun 18, 2013

tomdavies commented Jun 18, 2013

So the final question remains: would you like to amend your PR to have a named module (and squash commits ideally), or would you like me to bin it off and do it myself?

WickyNilliams commented Jun 18, 2013

off and do it myself?

WickyNilliams commented Jul 4, 2013

tomdavies commented Jul 5, 2013

WickyNilliams commented Jul 5, 2013

mhrisse commented Jul 15, 2013

WickyNilliams commented Jul 18, 2013

tomdavies commented Jul 19, 2013

WickyNilliams commented Jul 22, 2013

WickyNilliams commented Jul 22, 2013

WickyNilliams commented Jul 30, 2013

mhrisse commented Jul 31, 2013

tomdavies commented Jul 31, 2013

mhrisse commented Aug 21, 2013

WickyNilliams commented Aug 21, 2013

chiplay commented Nov 6, 2013