Coverage remained the same at 100.0% when pulling 6735cfa on natevw:nvw-checkPropTypes into df926b3 on developit:master.

Awesome, that's an excellent start 👍 A few minor changes + some tests cases are missing and then I think it will be ready to go in 🎉

Regarding your questions:

Questions/disclosures:

The file "debug/test/browser/debug.test.js" still needs the real 'prop-types'; I'm assuming that it will have that via "debug/package.json" somehow when the tests are run? If not, then I imagine the simplest fix would be to make prop-types a top-level devDependency.

yup, to be able to test against prop-types we need to declare it as a devDependency. Note that from npm's point of view there is only one package which is why it must be added to the main package.json file, not the one in debug/package.json.

The original version was ± ES3. (...)

Our build tool microbundle will automatically transpile it down to ES5 🎉

The original version does its own validation on both the typeSpec function and its return value, essentially validating the validators and printing helpful messages if it finds them wanting. I've removed that and just let the raw fallout get logged instead.

Good call! Agree, I don't think we need to support that use case. There was a time where devs could write their own prop-type checkers but I haven't seen any of those in the real world so far 👍

Oh, also I wasn't sure whether the implementation belonged in debug/src (next to e.g. "constants.js") or in debug/src/devtools (next to e.g. "custom.js") so I just went with the parent folder.

It belongs in debug/src. The code in debug/src/devtools is specifically for the react-devtools extension. So you made the right choice by intuition 👍

Let me know what you think 🙂

Thanks, appreciate the feedback. I can make some of the tweaks right now, but it will take me a bit longer to get up to speed with the test/coverage improvements (likely not until after April 15…).

Okay, inline feedback addressed but there's I think two remaining threads from #1525 (comment) here:

yup, to be able to test against prop-types we need to declare it as a devDependency

Right now prop-types is not a devDependency of the top-level package but my own local npm test and the travis-ci integration here seem to be happy 😬

If this is just some caching side-effect (I did rm -r node_modules debug/node_modules before starting to test earlier) then we should probably add it as a top-level devDependency to be safe? But that seems a bit questionable if it already works fine without?

Otherwise the followup here would be to move prop-types in "debug/package.json" to be a devDependency since even there it is only needed for tests.

As far as coverage, it's a lot better after the recent commits, -0.1% instead of -2.04% but especially since there's already tests for this should be relatively easy to trigger the missing line:

PropTypes
✔ should fail if props don't match prop-types
✔ should not print to console when types are correct

Maybe I'll just look into that while I'm on a roll anyway!

Ah, figured out the "why do the tests work" bit too. prop-types is currently getting installed anyway via:

├─┬ eslint-config-developit@1.1.1
│ └─┬ eslint-plugin-react@7.12.4
│   ├─┬ prop-types@15.7.2

Which is only coincidental.

I think all the current concerns have been addressed; I'm happy with where this is at and it's ready for review again.