Work in Progress: New build system and cleanup

[robwierzbowski]

OK, here we go!

I had a chance to go through the whole system. Cleaned up some unused files, reorganized, and rewrote the build system as promised.

This changes the API, which I'd like to get your feedback on. The cafe I'm working at is closing, so I'll expand a little on the benefits/reasoning tomorrow. Also, tests are currently broken but I'd like to nail down the API before I fix them.

The positive code is mostly comments, readme, and modularization. And the new CLI.

Can you explain what the use of generate-meta is? I've broken it a little and will fix up; knowing it's purpose will help with that.

Everything's documented in the readme.md. More tomorrow.

[patrickkettner]

I don't think this is nessecary

[robwierzbowski]

No problem.

[patrickkettner]

no matter what happens, bravo sir. I am impressed.

cc @SlexAxton @ryanseddon @stucox

[patrickkettner]

it'd be great to actually document the help command here

[robwierzbowski]

Sure. I didn't want to have too much code duplication, but no problem.

[robwierzbowski]

Some questions:

API Changes

I've changed the API a little: the node package outputs only a single file at a time, there's an options object (altered type signature) which includes a destination or false to output no file, and the callback has a single argument. My goal was to have less side effects, more specifying what you want the function to do. Also moved all non-Modernizr output options out of the config object and into the options argument.

What do you think about taking it a little farther and making the node package only return string output in the callback? The CLI and Grunt plugins would still output files, but when you're coding node you can use whatever file writing or data storage method you like to handle the modernizr output. This would let us remove the dest option entirely. A little more decomposition to make each piece of Modernizr workflow do one thing and do it well, without trying to guess how or what people want to do?

Defaults?

Right now a default Modernizr configuration doesn't exist, unless you're in the cli. What do you think about having an all option that outputs the most complete current dev version? Maybe just an all string as an alternate to the feature-detects array so that people can still set classes if they need to?

Verbose flag?

How much success/error logging should there be? I included one try/catch block when writing the file (in case there are permission issues, for example) and assume that everything else will throw an error that will bubble up unless the user specifically catches it when calling modernizr.build(). Anywhere else a custom error/success message would be helpful?

Other little stuff

Every feature detect in the options object is prefixed with test/, which doesn't really mean anything in the config context. Should we prepend that in generate-init.js?

[robwierzbowski]

Today I'm going through the tests to get them working again. A couple questions:

The API tests are pretty basic, looking good there. test/api/index.js can probably be blown away: it's testing if a modernizr main file exists, and if it has a function. IMO there's no need to test the first, as all future tests would fail if that returned false, and test/api/build.js tests the second part in a more specific and useful manner.

I could use a little clarification on caniuse.html and modular.html. It looks like caniuse.html is there for visual/human reference only. True? Modular.html re-implements a require build in browser. Is it just there for human preview? Since it uses a separate, checked in version of r.js, the built code may differ from what's built by the updated r.js in node_modules/requirejs. I could browserify the build system in lib/ and load it in modular.html, but in order to see the code built in the browser you have to download the repo anyways, at which point you could just build with the cli. If it's not valuable. removing the file would reclaim some complexity and maintenance costs.

[paulirish]

It looks like caniuse.html is there for visual/human reference only. True?

yup!

[robwierzbowski]

If you'd like any of the changes I'm making while getting tests working moved into their own PRs or reverted, let me know. Build rewrite is a pretty big change, and while I'm touching things I see a lot of stray files/unused config/etc.

[patrickkettner]

anything unrelated to the new build PR should be in a separate one.

thanks a ton dude!

On Sun, Jan 12, 2014 at 3:27 PM, Rob Wierzbowski
[email protected]:

If you'd like any of the changes I'm making while getting tests working
moved into their own PRs or reverted, let me know. Build rewrite is a
pretty big change, and while I'm touching things I see a lot of stray
files/unused config/etc.

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

patrick

[robwierzbowski]

Gruntfile is heavily changed by rewrite, so I'll leave those changes in. Moving the stray file cleanup from src to another PR.

[SlexAxton]

A few notes up top:

• Looking pretty good. A lot of files were still left around from the 2 or 3 large structural changes in Modernizr. Happy to have most of them removed
• We can't forget about the browser case. So it will be important in the release version of modernizr to share as much code with the client side builder as possible. ( http://v3.modernizr.com/download/ as a reference ) -- note: I'm not particularly interested in using Browserify and AMD to make this happen. Require.js is already required on the build page (CJS style is fine for /lib stuff), so it'd be good to keep it there.

What do you think about taking it a little farther and making the node package only return string output in the callback? The CLI and Grunt plugins would still output files, but when you're coding node you can use whatever file writing or data storage method you like to handle the modernizr output. This would let us remove the dest option entirely. A little more decomposition to make each piece of Modernizr workflow do one thing and do it well, without trying to guess how or what people want to do?

I'm all for this idea. The 'grunt can do files' vs. 'the cli tool just does streams' is a good one, I think.

Right now a default Modernizr configuration doesn't exist, unless you're in the cli. What do you think about having an all option that outputs the most complete current dev version? Maybe just an all string as an alternate to the feature-detects array so that people can still set classes if they need to?

Cool with a default set. It was in my original spec for the CLI tool, and I think we can add some other flags along the way as well top10, css3ish-stuff, 2.6.1devtests

How much success/error logging should there be? I included one try/catch block when writing the file (in case there are permission issues, for example) and assume that everything else will throw an error that will bubble up unless the user specifically catches it when calling modernizr.build(). Anywhere else a custom error/success message would be helpful?

As long as it's under a flag, the more logging the better. Show no restraint.

Every feature detect in the options object is prefixed with test/, which doesn't really mean anything in the config context. Should we prepend that in generate-init.js?

I think I'm cool with this. It breaks some consistency, but I think it's ok. Not too many people are going to think about it in the sense of a file system anyways.

Can you explain what the use of generate-meta is? I've broken it a little and will fix up; knowing it's purpose will help with that.

generate-meta as you have potentially gathered by some of your changes is to pull all the metadata that we have in each test and output a json block that we can use in the build tool. This is so we can have rich information and documentation pulled into the builder, but be directly written in the test files (where people are directly able to contribute). Likely a small webapp will pull in the output of generate-meta into a collection of test models and show and sort them onto a page.

[robwierzbowski]

@SlexAxton Thanks, that clears a lot of things up. Working tests and revisions coming soon!

[robwierzbowski]

I'm all for this idea. The 'grunt can do files' vs. 'the cli tool just does streams' is a good one, I think.

Done.

Cool with a default set. It was in my original spec for the CLI tool, and I think we can add some other flags along the way as well top10, css3ish-stuff, 2.6.1devtests

Currently the node package requires a config object, and the CLI outputs a "dev" file with all feature detects if it isn't given one. I'm happy to switch between all or nothing anywhere in here PR, and let's tackle named defaults in a separate PR.

As long as it's under a flag, the more logging the better. Show no restraint.

I didn't implement much logging, only success messages on the CLI. Suggestions welcome, not sure what else is needed.

Node unit tests are working. I'm going to take a look at generate-meta tonight.

[stucox]

cc @doctyper also – need to make sure these changes work for grunt-modernizr

[doctyper]

I'm making headway implementing these changes in grunt-modernizr. I really like that Grunt has been abstracted away. I'm using it as an opportunity to make grunt-modernizr agnostic as suggested in Modernizr/grunt-modernizr/issues/54.

@robwierzbowski So far the big missing piece is the metadata parameter in the CLI. I make heavy use of it to map and parse out test names in files.

[robwierzbowski]

Glad you like it! The metadata generation is finished, just need to
document and push.

On Monday, January 13, 2014, Richard Herrera wrote:

I'm making headway implementing these changes in grunt-modernizr. I really
like that Grunt has been abstracted away. I'm using it as an opportunity to
make grunt-modernizr agnostic as suggested in Modernizr/grunt-modernizr#54Modernizr/grunt-modernizr#54
.

@robwierzbowski https://github.com/robwierzbowski So far the big
missing piece is the metadata parameter in the CLI. I make heavy use of it
to map and parse out test names in files.

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

Rob Wierzbowski
@robwierzbowski http://twitter.com/#!/robwierzbowski
https://github.com/robwierzbowski
http://robwierzbowski.com

[robwierzbowski]

Got generate working again, cleaned it up a little and added it as a method off of modernizr object and as a flag on the CLI. The output is compressed, so looks a little different from the previous. I beautified and diffed with the original and there are no content differences, only formatting.

[robwierzbowski]

@doctyper: I revised the modernizr build function signature to make it a little more standard (modernizr.build(config[, opts], callback)).

Last thing to do: Browser tests and in-browser generation

OK, almost finished :D. Sorry for the long running PR, this month I've only had the weekends to work.

When I started I accidentally broke in-browser generation: I thought that generate-init.js was clearly part of the build system and moved it to lib/ and node-style require. Now I see that it's being used in-browser with in-browser r.js to generate a custom build on the modernizr page.

A couple questions (for @patrickkettner, @doctyper, @stucox, @SlexAxton, @paulirish in particular):

What are the responsibilities of this repo? It

• Holds all Modernizr.js source modules
• Tests feature detects
• Is a node package that builds a custom Modernizr build
• Exposes the node package as a CLI
• Is a dependency of other node-based projects, like Grunt-modernizr and possibly gulp-modernizr

Does it also

• provide the feature detects and require.js modules for the web based builder at Modernizr.com? (yes, I think so)
• provide an in-browser build system for Modernizr.com? (I'm not sure)

It looks like the configuration (r.js) and a lot of the logic is inlined and duplicated.

Let's define explicit responsibilities for these two repos. Should Modernizr (this repo) contain both a node and browser based build system, and modernizr.com uses the latter with no code duplication? Should Modernizr hold only the node based build system, and Modernizr.com pulls Modernizr in for source but holds the browser based build system (this would make sense if no one else is likely to build modernizr in the browser)? Or should the repos be further decomposed into modernizr-source.git, modernizr-node.git, modernizr-browser.git, modernizr.com.git?

I'm going to start working on # 1 for now, which could easily be translated into # 2 — I get really nervous when a PR runs for a month 😄. But I love the decomposition @doctyper has done with grunt-modernizr, breaking the file parsing responsibility into its own node project, and I thought it was worth thinking about here.

[robwierzbowski]

Actually, I think I need some input on those questions above before I proceed. Should this repo be primarily an in-browser build system that's wrapped in node for node.js and cli usage, or should it be primarily a node build system with consumable parts for a separate in-browser build system?

[patrickkettner]

Should this repo be primarily an in-browser build system that's wrapped in node for node.js and cli usage, or should it be primarily a node build system with consumable parts for a separate in-browser build system?

I would think the best thing would be the former, if for no other reason than the fact that the browser testing has to be in this repo. That being said, I think your own input would be very valuable in informing me, at the very least.

[doctyper]

To add to the separate-repo argument, a smaller npm package makes it less annoying for developers to use as a dependency. When I build node packages, I tend to look for alternatives whenever one dependency takes too long or makes too much noise on install. I'm all for shaving ms wherever possible on package install time.

Another point to consider is versioning. This npm package will eventually need to version bump with every push to master. It would be less confusing to split the repo based on intent. Otherwise, you might version bump to fix a browser bug or version bump to fix a CLI bug.

[robwierzbowski]

Totally agree. My ideal would be:

• This repo as an in-browser library, which can be pulled in by the web site as a build system and by...
• A separate repo that wraps the browser library in a node-based build system and a node based cli

I think it would be the same amount of work from here as solving the problems in a single repo, with a better final outcome.

[robwierzbowski]

OK, back on this today.

Note to self: normalizing naming of feature tests and files would make testing easier.

[robwierzbowski]

Going to try creating responsibilities like this:

Modernizr src/ is a require.js based script that creates a full Modernizr build based on a config argument.
A new js module that wraps r.js takes require input and outputs a built file.
That module can be included in a web page or wrapped in a node module.
Node module also provides CLI. Can be split into its own module in the future.

[patrickkettner]

sgtm

[patrickkettner]

ping @robwierzbowski - lets do this, mang. I'm free whenever you are - I want to ship yer stuff

[robwierzbowski]

Sorry, let this languish. I can make time to do a bit of work this weekend.
I think we can break this down into little PRs and get some of the good
stuff out quick.

Want to gchat on Saturday?

On Tuesday, June 17, 2014, patrick kettner [email protected] wrote:

ping @robwierzbowski https://github.com/robwierzbowski - lets do this,
mang. I'm free whenever you are - I want to ship yer shit

—
Reply to this email directly or view it on GitHub
#1184 (comment).

Rob Wierzbowski
@robwierzbowski http://twitter.com/#!/robwierzbowski
https://github.com/robwierzbowski
http://robwierzbowski.com

[patrickkettner]

yes, yes I do :D

[robwierzbowski]

Nice. I'll take some time this week to break up some tasks, and then Sat
let's talk and either divide and conquer or pair!

Rob Wierzbowski
@robwierzbowski http://twitter.com/#!/robwierzbowski
https://github.com/robwierzbowski
http://robwierzbowski.com

On Tue, Jun 17, 2014 at 9:12 AM, patrick kettner [email protected]
wrote:

yes, yes I do :D

—
Reply to this email directly or view it on GitHub
#1184 (comment).

[patrickkettner]

huzzah!

[robwierzbowski]

OK, I got totally eaten by wedding recuperation and house cleaning on Sat, sorry about that. Up for rescheduling whenever you have some time.

List of things that we can break out into separate PRs:

1. Removing orphan and duplicate files. There were a bunch when I started this PR, not sure if any remain in current master.
2. Refactor to a generic require based build sys that we can use on the site and in a node plugin. I did some work originally making the build sys pure node, before I realized that it needed to work on modernizr.com too. Tests should all run against this pure JS build sys, and it should be completely bundled into a single file that reads config from an object passed in, or maybe a global. IIRC, there were ie8 concerns at one time that made this difficult, but over the past few months it's become de rigueur to drop that browser like a sack of potatoes.
3. Re-implement the node wrapper for the universal build sys. This is where we can clean up the gruntfile too, and add in the improved test coverage in one of the above commits
4. Re-implement the CLI wrapper for the node wrapper :)
5. Make nice package.jsons and bower.jsons so projects can require the node module to build as part of a grunt / gulp plugin or as a require module so they can build in browser (although no one should do that except modernizr.com).

At the end we should have a more streamlined build sys that's nicely separated into universal base, node, and CLI parts. I can help with a bunch of this, but probably not all of it.

Whatcha think?

[doctyper]

There has been a lot of recent gulp-modernizr bug reports. Some/most of these are dependent on the status of this pull request.

@robwierzbowski @patrickkettner I'm ready and willing to carve some time out this weekend to land this PR. Can we make it happen?

[patrickkettner]

Totally my fault for not responding sooner - had a wedding (as well!) and I/O, and was a lot busier than anticipated.

Totally down for this weekend. Do you guys want to meet up on chat/hangout/irc or something to make a gameplan quickly? Im free from ~7 hours from now onwards

[robwierzbowski]

We're all so busy :). I can do Saturday morning (east coast), but tonight
and tomorrow are off-computer time.

Rob Wierzbowski
@robwierzbowski http://twitter.com/#!/robwierzbowski
https://github.com/robwierzbowski
http://robwierzbowski.com

On Thu, Jul 3, 2014 at 12:26 PM, patrick kettner [email protected]
wrote:

Totally my fault for not responding sooner - had a wedding (as well!) and
I/O, and was a lot busier than anticipated.

Totally down for this weekend. Do you guys want to meet up on
chat/hangout/irc or something to make a gameplan quickly? Im free from ~7
hours from now onwards

—
Reply to this email directly or view it on GitHub
#1184 (comment).

[patrickkettner]

:D - saturday morning works for me, @doctyper ?

[doctyper]

I'm free Sunday and a possible bonus day Monday. But don't let me stop the game-plan, I can hop on to whatever shakes out.

[robwierzbowski]

Is everyone east coast? Or are these mornings different mornings?

Rob Wierzbowski
@robwierzbowski http://twitter.com/#!/robwierzbowski
https://github.com/robwierzbowski
http://robwierzbowski.com

On Thu, Jul 3, 2014 at 12:57 PM, Richard Herrera [email protected]
wrote:

I'm free Sunday and a possible bonus day Monday. But don't let me stop the
game-plan, I can hop on to whatever shakes out.

—
Reply to this email directly or view it on GitHub
#1184 (comment).

[patrickkettner]

You and I are, @doctyper is cali, unless hes traveling.

Wanna meet up saturday morning @robwierzbowski, and then we can catch @doctyper up on sunday?

[robwierzbowski]

Hells yes. 10am?

On Thursday, July 3, 2014, patrick kettner [email protected] wrote:

You and I are, @doctyper https://github.com/doctyper is cali, unless
hes traveling.

Wanna meet up saturday morning @robwierzbowski
https://github.com/robwierzbowski, and then we can catch @doctyper
https://github.com/doctyper up on sunday?

—
Reply to this email directly or view it on GitHub
#1184 (comment).

Rob Wierzbowski
@robwierzbowski http://twitter.com/#!/robwierzbowski
https://github.com/robwierzbowski
http://robwierzbowski.com

[patrickkettner]

created the build-refactor branch to PR all of the future changes against.

[robwierzbowski]

Notes from dipping into the code again today:

Goals:

Require based build system (which is 90% there) that can be used in the browser and in node. Its interface should be a single function that takes a config object and an environment object/function.

Node package and CLI for interacting with the require based build sys.

Changes

• do rjs config in require build system. The function exposed to the browser will need to be a wrapper for the rjs builder function.
• build banner internally. Node should build the same banner in case people want to copy/paste into the browser.
• every option needs to be specified in config.
• Add environment object/function that contains everything that needs to be different between the two environments. Looks like mostly rjs's out parameter.

Catch up

• it looks like these don't have a place in config currently: https://github.com/Modernizr/v3.modernizr.com/blob/gh-pages/i/js/builderapp2.js#L243

First steps:

Add rjs as the entry point to the current build system, find the options that aren't currently in the cofig and add them

Side notes

• Make sure tests are well separated. Tests for built modernizr itself, tests for the require builder, tests for the node builder
• Separate node builder into its own directory, so it can easily be moved to its own repo

[patrickkettner]

sup @robwierzbowski

[SlexAxton]

My gut is that we should probably just rebase this branch into master so we're all working on the same code. I think it's probably already in a better state than the current master build system, so no need to just make the PR bigger.

[patrickkettner]

@SlexAxton - this branch's grunt is currently broken, so I would disagree that merging to master is a good idea. Maybe a branch on the modernizr account, though?

I think creating a checklist of all the crap that needs to happen and saying what individuals are working on would be a decent idea

[SlexAxton]

Let's just fix the jshint error or whatever that is and get it in? or is it worse than that?

[patrickkettner]

it was like totally hosed last I checked, but that was a while back and am mobile at the moment. if its easy enough to have it "working" then 👍

[robwierzbowski]

I believe it is totally hosed. Basically there are parts of it that need to
be rewritten and a function that wraps the rjs builder so it can be used
for both the site and the node app. Honestly and obviously I have too much
on my plate to work on it now.

On Wednesday, August 27, 2014, patrick kettner [email protected]
wrote:

it was like totally hosed last I checked, but that was a while back and am
mobile at the moment. if its easy enough to have it "working" then [image:
👍]

—
Reply to this email directly or view it on GitHub
#1184 (comment).

Rob Wierzbowski
@robwierzbowski http://twitter.com/#!/robwierzbowski
https://github.com/robwierzbowski
http://robwierzbowski.com

[SlexAxton]

Totally cool. Should we just abandon this attempt or is there stuff worth pulling out here? Do you have any state dump on what specifically is hosed? No worries on being busy, just wanna pick back up where we left off.

[robwierzbowski]

I think the best idea would be to close this branch and continue on the
main branch. Anyone that wants to work on streamlining the testing /
building system is more than welcome to message me.

We're getting a half day a month at my company to work on outside projects;
I have a couple ahead of modernizr, but if/when I catch up I'll head back
here and get cracking on more incremental PRs.

On Wednesday, August 27, 2014, Alex Sexton [email protected] wrote:

Totally cool. Should we just abandon this attempt or is there stuff worth
pulling out here? Do you have any state dump on what specifically is hosed?
No worries on being busy, just wanna pick back up where we left off.

—
Reply to this email directly or view it on GitHub
#1184 (comment).

Rob Wierzbowski
@robwierzbowski http://twitter.com/#!/robwierzbowski
https://github.com/robwierzbowski
http://robwierzbowski.com