Move all "core" detects outside of modernizr.js #486

Closed
paulirish opened this Issue Feb 7, 2012 · 30 comments

Comments

Projects
None yet
10 participants
Owner

paulirish commented Feb 7, 2012

There is no "default build". Sorry CDNs.

All tests will live in /feature-detects/

DEPENDENCY: #487

Owner

SlexAxton commented Feb 8, 2012

I'd imagine we will want to adopt some sort of dependency management system. Just a heads up to start thinking about this.

Currently I do this all by hand in the modulizr file, that's likely not scalable.

Contributor

josh commented Feb 9, 2012

❤️

I've been wanting to ship a Rails/Sprockets Modernizr gem that acted like the download builder. Which would basically allow you to do something like:

// application.js
//
//= require modernizr/canvas
//= require modernizr/history
//= require modernizr/websockets

You guys probably don't want to use Sprockets yourselves.

I do like the current format of the flat feature-detects/ directory. It'd be easy for me to work with.


Paul's Edit: josh published his repo: https://github.com/josh/ruby-modernizr

Contributor

addyosmani commented Jun 25, 2012

Just wanted to check and see if anyone had started working on this. Might be able to pitch in some time on it in a few days if not :)

Owner

paulirish commented Jun 25, 2012

not yet nope.
the help would be great

Contributor

jokeyrhyme commented Jul 24, 2012

Sort of a related question (but please berate me for derailing this issue), but how does the builder work with respect to GitHub? Does it pull in the latest tagged "stable" release? Or is it always just master/HEAD?

Once these feature-detects are broken out of the core, how will the answer to the above be different? In an ideal world where the future long-term vision for Modernizr is realised, how would the answer be different to the above?

I guess another way to ask this would be: will the contents of the feature-detects directory be stabilised via tags? Or will the builder assume they are "living" and just grab the latest code?

The reason I ask, is I'm responsible for managing our CDN copy of Modernizr, and I've been struggling to "version" the filename. I've tried using the core version number with the year+week of when I downloaded it, but would the GitHub commit hash be better?

If the builder only ever deals with tagged versions (and will continue to do so), then I can simplify by just using the tag's version in my filenames.

Owner

paulirish commented Aug 5, 2012

@jokeyrhyme we manually have it pull in the latest stable version of modernizr.js and then we manually update the "addon detects" to whatever's in HEAD at the time.

Once these feature-detects are broken out of the core, how will the answer to the above be different? In

good question. I think we'll go with a "living" state but we'll have to have versions somehow...

for now the first 7 letters of the SHA would be a great mechanism for version. i dont think we'll use git tags going forward for versioning.

maybe something like 3.0.fa4be9a

is that crazy?

Contributor

jokeyrhyme commented Aug 5, 2012

@paulirish not too crazy, but I like things to be automatically sorted chronologically. I made a comment elsewhere about the filename the builder generates: #287 (comment)
Thanks for letting me pick your brain.

Owner

paulirish commented Aug 7, 2012

just to be a little more verbose:

  • so my current proposal is something like
  • 3.0.fa424ae
  • and that refers to the most recent commit to master that is captured by this build
  • and sure the filename would be modernizr-3.0.fa424ae.js

.

alternatively, we could use automation so its like 3.0.121 and we have a mapping done somewhere to map these semver versions to the SHAs in master they represent. We would bump on changes to all the .js files but ignore readme and testsuite stuff.

I believe @KuraFire prefers the latter proposal here.

Owner

KuraFire commented Aug 7, 2012

From briefly chatting with @bobthecow about this, I got the impression that we'd need to run a cron on a server to do automatic tagging, but he made the good point that we don't want to expose each change to the code as a version to the outside developers using Modernizr, either.

What we could do is something based on tagging a version only when we want to publish a new release, and use the SHA of the tag. That way, it's still a manual process, but we also don't care about (or have to deal with, really) exposing tiny, individual and incremental changes into the public filename/version consciousness.

To further elaborate what that would mean in @paulirish’ syntax:

  • 3.0.fa424ae for a release
  • fa424ae is the SHA of the manually cut tag that we make for a specific public release of Modernizr
  • 3.0 is the named version we publish, i.e. the "version in the vernacular"; meaning we no longer do minor version numbers, those are replaced by SHA tag commit hashes.
Member

bobthecow commented Aug 7, 2012

The problem with using a SHA as a version number segment is that it's impossible to sort. Does 3.0.ac2f1a come before or after 3.0.9a6e5c?

Owner

KuraFire commented Aug 7, 2012

We wouldn’t do that, we would always bump the medium for new releases, so 3.0.ac2f1a and then 3.1.9a6e5c.

Update: I guess my earlier comment does not make that very clear.

Member

bobthecow commented Aug 7, 2012

Then what's the point of including the SHA in the version number? It's trivial to check what SHA a tag points to, if you really want to know. I'd prefer a more normal (and semver compliant) v3.0.1.

Owner

KuraFire commented Aug 7, 2012

My understanding was that this was for third-party systems (CDNs etc.) that want to support multiple versions in an automated way.

Owner

KuraFire commented Aug 7, 2012

…and who want to be able to just point to a single atomic component for a version, which, with 3.0.1, would require someone mapping 3.0.1 to the SHA of the release (tag).

Member

bobthecow commented Aug 7, 2012

you mean like a git tag? that seems like a completely valid way of mapping a version to a SHA.

Owner

KuraFire commented Aug 7, 2012

But not ‘automatable’ for third parties, unless I’m missing something?

Owner

paulirish commented Aug 9, 2012

Alright so automated git tagging (retaining semver) would be ideal. Bump on
changes to .js only.
And we manually change the minor/major with some config.

Contributor

rupl commented Sep 19, 2012

For Drupal 8 we have a strong desire to automatically generate Modernizr builds from the current state of a modular PHP codebase. There's also a desire to inline individual feature detects for one-time use, and it seems that resolving this Modernizr issue would make big progress towards that type of functionality for numerous modular platforms (that Ruby gem was mentioned above too). I'm sure someone else will wander over here but I wanted to express interest in helping.

A couple questions:

  • In modernizr.js I see the section called Tests accompanied by comments like /*>>mq*/ — Are these the regex hacks that @SlexAxton wants to get rid of?
  • Do you aim to make the core feature detects work like the community tests do right now? Or is there a bigger plan?
Owner

paulirish commented Sep 19, 2012

I talked to @rupl and @seutje on hangout just a bit ago.. these are chris's notes which are awesome.

Modernizr 3.0

  • Moving core feature detects out of modernizr.js -- It may not be too hard to do this.
  • Every helper method inside the JS file is now accounted for in the plugin API
  • Porting things out into their own JS files is not too hard
  • Alex brought up dependencies - could be hard.

Todos

  • dependency approach
    • 15 or 20 tests - some depend on other tests
    • webgl extensions test depends on webgl
    • canvas-text depends on canvas
    • We haven't defined semantics for dependencies, right now we manage in some JSON in the builder code -- not ideal.
    • Ideally we get a better way to define dependencies -- may not be a blocker
  • 2 or 3 hour sprint could pave the way for most of this work.

Next steps

  • Make decision about dependencies (AMD, slex??)
  • Do the port
  • Versioning: basic idea is they need a bit more automation for making a new tag and new version every time a change to .js is committed to master. Right now they tag releases, they'd rather just ship master constantly, relying on Travis passing the build.

Converting the 15-20 tests

Owner

SlexAxton commented Sep 19, 2012

My proposal is to switch everything over to AMD, but work out how to "build out" the cruft from each test. Gzip should take care of most of that stuff, but this will stop people from complaining and shouldn't be too hard. It is a bastardization since we'll still be dealing with globals, etc. But I think it's a good compromise:

// Tests if your computer can hover
define('hovertest', ['fantest'], function (/*don't actually use anything here*/) {
  Modernizr.hovertest = Modernizr.fantest && window.document.hoversauce === 'forealz';
  // don't actually return anything.
});

Then I can pretty much use r.js to run a build, and then strip out the function wrappers without worrying about ruining the insides.

Seem good?

Owner

ryanseddon commented Sep 20, 2012

+1 for AMD I think that is a nice approach to handle deps. As for tests using internal APIs easy to just switch them over to the exposed ones.

Plus this will be a good kick up the bum to get the code convention/guidelines doc up.

Contributor

jokeyrhyme commented Sep 20, 2012

Is there a separate issue for the dependency work? If there is going to be an new API, it's probably worth tackling asynchronous tests (#622) at the same time. You certainly wouldn't want to do anything to make that impossible later.

I noticed that the Cordova project (a.k.a. PhoneGap) uses AMD internally, and exposes it's own internal "require" method via: window.cordova.require. This makes it easy to add plugins via AMD without potentially conflicting with AMD in the rest of the JavaScript. Is it worth doing something similar with Modernizr?

Contributor

addyosmani commented Sep 20, 2012

+1 for AMD.

Contributor

seutje commented Sep 21, 2012

I'm a little confused about the preferred formatting, some tests simply pass a boolean (style-scoped for instance) as second parameter to addTest, and some pass a function that returns a boolean (window-framed for instance). What does this depend on?

Owner

paulirish commented Sep 22, 2012

, some tests simply pass a boolean (style-scoped for instance) as second parameter to addTest, and some pass a function that returns a boolean (window-framed for instance)

Basically we added som extra method signatures to addTest to skip creating a function when it was unnecessary.
http://modernizr.com/docs/#addtest

Owner

paulirish commented Sep 22, 2012

I'm down for AMD for this..

I 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).

Let's try this out with a real example.

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

Just for fun, here it is with CJS-style AMD.

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

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

feels good man.


(just a mental note that we'll probably mass-rename all the files, so their filename matches their property name. and no dashes)


Contributor

rupl commented Sep 24, 2012

Ok we've moved the bulk of the tests over, but I have a couple questions

  • rgba and hlsa: can the functions using setCss be moved over the same way? We moved them but weren't positive it was the correct thing to do.
  • webforms: huge function which is then assigned directly to the Modernizr object instead of using the tests array. Can this be treated the same as the others?
  • grunt's lint task is reporting many function undefined errors as a result of moving the bits of code out of modernizr.js which holds all the other functions. Fix needed or ignore safely for now? Or perhaps I can alter the grunt file? I'm new to grunt plz advise.

Also, AMD sounds like a go, but do we want AMD taken care of in this same issue?

Owner

paulirish commented Sep 26, 2012

holy crap!! everyone, check out the amazing work @rupl and @seutje did! ==> https://github.com/rupl/Modernizr/compare/master


setCss

  • setCss is a handy internal method for applying css to an off-dom method so we can pull it back out again and see if it stuck. unlike Modernizr.testStyles() which adds a dom element into the page so you can getComputedStyles on it.
  • usually testing document.documentElement is good enough for the soft checks but we probably dont want to use it here. three options:
    1. do nothing. we're just forced to make a dom node each time for this check.
    2. expose a new Modernizr._elem which is off-dom and we can use for things like this.
    3. allow testStyles() to do off-dom stuff?

I'm gravitating towards 1. creating new elements is cheap.

webforms

  • it was assigned to Modernizr because the tests array is run through and executed and the return assigned to the Modernizr object. input and inputtypes could and should be separated.

grunt

Can we solve it with adding globals to the jslint options? Ignore for now.

AMD

Let's do separate issue. Good call. Can you make a new issue that opens discussion summarizing this to 7 comments after?

Contributor

jokeyrhyme commented Sep 27, 2012

How should an additional input or inputtypes tests be written? What should they depend on?
For an example test (say, one that populates Modernizr.inputtypes.file):

  • should it first check to see if Modernizr.input exists and create it if not?
  • should it depend on something that initialises Modernizr.input?
  • should Modernizr core always initialise test categories/objects like input and inputtypes?

Is it worth adding new container objects like 'Modernizr.dom', '.css', etc? This would break backwards-compatibility...

Owner

paulirish commented Sep 27, 2012

new input/inputtypes (see #710 btw!) tests will just depend on the base ones, and we'll use AMD to define that dependency.

@paulirish paulirish closed this in f5060c0 Oct 15, 2012

paulirish added a commit that referenced this issue Oct 15, 2012

Merge pull request #712 from rupl/master
Fixes #486: moving core feature detects into individual files

patrickkettner pushed a commit to patrickkettner/Modernizr that referenced this issue Feb 22, 2015

patrickkettner pushed a commit to patrickkettner/Modernizr that referenced this issue Feb 22, 2015

Merge pull request #712 from rupl/master
Fixes #486: moving core feature detects into individual files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment