Cache key doesn't take --source-map into account

[tomdale]

Expected Behavior

nyc's cache would be invalidated when different flags are passed.

Observed Behavior

Toggling source maps off and on doesn't have an effect; the result from whatever option was used the first time the command was run is used.

Bonus Points! Code (or Repository) that Reproduces Issue

glimmer-perf-app :: (master*) » nyc --no-source-map node dist/app.js                                                                                                                                                                                                                ~/Code/glimmer-perf-app
----------|----------|----------|----------|----------|----------------|
File      |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
----------|----------|----------|----------|----------|----------------|
All files |     43.7 |     23.3 |    38.23 |    43.73 |                |
 app.js   |     43.7 |     23.3 |    38.23 |    43.73 |... 20258,20262 |
----------|----------|----------|----------|----------|----------------|


glimmer-perf-app :: (master*) » nyc --source-map node dist/app.js                                                                                                                                                                                                                   ~/Code/glimmer-perf-app
----------|----------|----------|----------|----------|----------------|
File      |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
----------|----------|----------|----------|----------|----------------|
All files |     43.7 |     23.3 |    38.23 |    43.73 |                |
 app.js   |     43.7 |     23.3 |    38.23 |    43.73 |... 20258,20262 |
----------|----------|----------|----------|----------|----------------|


glimmer-perf-app :: (master*) » nyc --source-map --no-cache node dist/app.js                                                                                                                                                                                                        ~/Code/glimmer-perf-app
---------------------------------------------------------------------------------------------|----------|----------|----------|----------|----------------|
File                                                                                         |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
---------------------------------------------------------------------------------------------|----------|----------|----------|----------|----------------|
All files                                                                                    |    35.25 |     27.7 |    29.27 |    37.64 |                |
 Code/glimmer-perf-app/dist                                                                  |      100 |       50 |      100 |      100 |                |
  index.ts                                                                                   |      100 |       50 |      100 |      100 |                |
  main.ts                                                                                    |      100 |      100 |      100 |      100 |                |
 Code/glimmer-perf-app/dist/config                                                           |      100 |      100 |      100 |      100 |                |
  module-map.js                                                                              |      100 |      100 |      100 |      100 |                |
  resolver-configuration.js                                                                  |      100 |      100 |      100 |      100 |                |
 Code/glimmer-perf-app/dist/ui/components/glimmer-perf-app                                   |      100 |      100 |      100 |      100 |                |
  template.js                                                                                |      100 |      100 |      100 |      100 |                |
 Code/node_modules/@glimmer/component/dist/modules/es2017/src                                |    85.71 |      100 |      100 |    85.71 |                |
  component-manager.ts                                                                       |      100 |      100 |      100 |      100 |                |
...

Forensic Information

Operating System: OS X 10.11.6
Environment Information: https://gist.github.com/tomdale/f572f3f94cdbe808eb0d9abfb64880e9

[bcoe]

@tomdale are you picturing that we'd perhaps bake the nyc config into the hash generated for the cache? This might be the simplest approach is that it might place quite a bit of cruft in the cache directory if you're fiddling with your settings;

Would it be worth enumerating a few flags that have the most noticeable effect on the cache, start simple?

[tomdale]

@bcoe That sounds like a great approach. I think we do something similar in the Ember build system: feed the serialized config POJO into the hashing function. If that's a tractable option here, it seems better than trying to whitelist config options on a case-by-case basis. Better err on the side of too much cache invalidation than too little. 😄

[novemberborn]

Just observed a similar problem with --instrument.

As an aside it might be worthwhile using package-hash for this, once nyc drops 0.10/0.12 support. @bcoe what are your thoughts on that?

[bcoe]

@novemberborn I'd be tempted to start by just extending our existing hashing function with @tomdale's suggestion; this logic was already pulled into its own internal module so this will be nice and simple

nyc has a lot of dependencies, which I've sometimes seen it criticized for. yargs also ran into this critique, and I've changed my design philosophy to be somewhat conservative regarding pulling in third-party modules -- I think on yargs I've figured out a good balance, and have definitely bumped into this criticism less.

tldr; I'd like to at least perform an audit of nyc's dependencies before I pull in too many more (I'm pretty sure there are some libraries from early on in the project that might be vestigial at this point). And I think that implementing a slightly better caching function for nyc should be pretty simple without needing to add any dependencies.

[tomdale]

I think this is straightforward to accomplish with Node's built-in crypto module:

crypto.createHash('md5')
   .update(JSON.stringify(options), 'utf8')
   .digest('hex');