Opened 9 years ago

Closed 8 years ago

#12431 closed enhancement (fixed)

Use feature detection with the has() pattern wherever possible

Reported by: Kris Zyp Owned by: Kris Zyp
Priority: high Milestone: 1.8
Component: General Version: 1.6.0
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by bill)

Feature detection is highly regarded as a more robust, future-proof technique for quality code. We should be using feature detection where possible. In addition, we specifically want to use the has() pattern for feature detection so that build processes can understand the branches and eliminate code for known features or non-features when built for certain browsers.

The has() pattern does not require a "has" module or any detection module, it is a coding convention that makes your feature detection visible to the build process. You should refer to the tests in https://github.com/phiggins42/has.js/tree/master/detect to ensure you follow the same naming of tests (and possibly copy the detection code), but Dojo won't have any dependency on has.js (nor does it need to).

The primary usage pattern should look like:

  define([...], function(...){
    var features = {
      "json-parse": typeof JSON != undefined && JSON.parse
    };
    function has(feature){
      return features[feature];
    }
    if(has("json-parse")){
       // if JSON.parse is available
    }else{
       // if JSON.parse is not available
    }
  });

Note, I would like to see us avoid having a separate "has" module if possible. While the source code for using a "has" module (with has.add function) might look shorter, it actually does not compress as well as the code above. Putting tests in separate modules also adds unnecessary complexity to our modules, keeping tests within the module that uses it makes our code much easier to read without having to look in separate modules for understanding implementation. If there are certain larger tests that end up being used multiple times, they may be put in a single module, but that shouldn't be the default.

Attachments (1)

has-array.diff (2.8 KB) - added by Kris Zyp 9 years ago.
dojo/_base/array.js converted to has() (with some other fixes)

Download all attachments as: .zip

Change History (13)

Changed 9 years ago by Kris Zyp

Attachment: has-array.diff added

dojo/_base/array.js converted to has() (with some other fixes)

comment:1 Changed 9 years ago by Kris Zyp

One question to consider in some of these conversions is if we should start making _base modules that have legitimate standalone exports available in the top level folder. For example, we could have dojo/array that exports an object hash of array statics:

require("dojo/array").forEach(function(...)..

comment:2 Changed 9 years ago by Kris Zyp

See #8111 for new JSON module using has() branching.

comment:3 Changed 9 years ago by bill

Description: modified (diff)

I know this is your preference for how to do has() testing, but it's not something we concluded on, see the dojo-contributors thread. Rawld and I responded to your post but you just never answered.

comment:4 Changed 9 years ago by bill

Also, the array.js patch is apparently not backwards-compatible. See the comments for the current code stating how the behavior differs from standard array methods, ex:

//	This method corresponds to the JavaScript 1.6 Array.indexOf method, with one difference: when
//	run over sparse arrays, the Dojo function invokes the callback for every index whereas JavaScript
//	1.6's indexOf skips the holes in the sparse array.

comment:5 Changed 9 years ago by Kris Zyp

(In [24488]) Move dom-addeventlistener to inside browser block refs #12431 !strict

comment:6 Changed 8 years ago by Kris Zyp

(In [25602]) Remove erroneous spaces in feature names, refs #12431 !strict

comment:7 Changed 8 years ago by Kris Zyp

(In [25661]) Change return value of sniff to has, #refs #12431 !strict

comment:8 Changed 8 years ago by Kris Zyp

(In [25681]) Update dom-geometry to use sniff correctly, #refs #12431 !strict

comment:9 Changed 8 years ago by Kris Zyp

(In [25682]) Revert has("quirks") back to dojo.isQuirks due to temp changes, #refs #12431 !strict

comment:10 Changed 8 years ago by ben hockey

(In [25683]) r25661 missed a possible path that returned dojo rather than has. refs #12431 !strict

comment:11 Changed 8 years ago by Kris Zyp

(In [25687]) Fix has() usage for sniff in dom-*, #refs #12431 !strict

comment:12 Changed 8 years ago by Kris Zyp

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.