Skip to content

Commit

Permalink
Leverage requirejs-plugins/has to conditionally load the Intl shim.
Browse files Browse the repository at this point in the history
  • Loading branch information
@clmath
clmath committed Jun 20, 2014
1 parent d94069f commit 9267dc0
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 4 deletions.
5 changes: 2 additions & 3 deletions IntlShim.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// Loads ./Intl.js only if necessary
//
/* global Intl */
define(typeof Intl === "undefined" || !Intl.NumberFormat || !Intl.DateTimeFormat ? ["./Intl"] : [],
function (ecma402Intl) {
return ecma402Intl || Intl;
define(["./features!intl-api?:./Intl"], function (IntlShim) {
return IntlShim || Intl;
});
3 changes: 2 additions & 1 deletion bower.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
"version": "0.1.0-dev",
"dependencies": {
"requirejs": ">=2.1.11",
"requirejs-text": ">=2.0.11"
"requirejs-text": ">=2.0.11",
"requirejs-dplugins": ">=0.2.2-dev"
},
"ignore": [
".jshintrc",
Expand Down
6 changes: 6 additions & 0 deletions features.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
define(["requirejs-dplugins/has"], function (has) {

This comment has been minimized.

Sign in to view

Copy link
@cjolif

cjolif Jun 20, 2014

Contributor

the jsdoc of this module must document the purpose and the flag

This comment has been minimized.

Sign in to view

Copy link
@wkeese

wkeese Jun 20, 2014

Member

I think also with how @JCEmmons has moved most of the private code to impl/ (and calendars/), maybe this file should be moved to impl/ too?

(In that case have JSDoc is not important any more?)

This comment has been minimized.

Sign in to view

Copy link
@cjolif

cjolif Jun 20, 2014

Contributor

I would tend to not move it. Indeed even if the user won't use it directly it will use the feature in it and needs a reference to the module to specify its configuration in RequireJS config section (to override the flag), so I would tend to keep it with the "visible" API, similarly to ecma402/locales.

This comment has been minimized.

Sign in to view

Copy link
@wkeese

wkeese Jun 20, 2014

Member

even if the user won't use it directly it will use the feature in it and needs a reference to the module to specify its configuration in RequireJS config section (to override the flag)

@cjolif, I think you're confused about how this works. You are thinking of something like this?

require.config({
    config: {
        "ecma402/features": {
            "Intl-api": false
        }
    }
}

Actually, that won't do anything, it needs to be this:

require.config({
    config: {
        "requirejs-dplugins/has": {
            "Intl-api": false
        }
    }
}

(Note that the module specified is requirejs-dplugins/has, not ecma402/features.)

This comment has been minimized.

Sign in to view

Copy link
@cjolif

cjolif Jun 20, 2014

Contributor

sorry, you are right about the syntax. That said I still think documenting our features (& sniff) modules with the has flags they contain is a good practice. This give our users a common way to know where to look for, for such things.
has.add("intl-api", function (global) {
return typeof global.Intl !== "undefined" && global.Intl.NumberFormat && global.Intl.DateTimeFormat;
});
return has;
});

0 comments on commit 9267dc0

Please sign in to comment.